Intelligently handle :pserver: paths for CVS repositories.

Review Request #6060 — Created July 4, 2014 and submitted

Information

Review Board
release-2.0.x
3eecf05...

Reviewers

Our CVS repository path handling was pretty bad. We only injecting the
form's username and password if not specifying a repository type (such
as :pserver:). This led to a lot of confusion.

Now, our regex and CVSROOT building are a lot smarter. The user can
provide a :pserver: (or equivalent) CVSROOT, and either include or leave
out the username or password. Those will then be provided based on the
form input.

The workaround for using hostname:port/path format has been removed from
the docs, since it's far more natural to use a valid CVSROOT.

Tested some combinations by hand against a real CVS repository, and looked
at debug output for the generated CVSROOT.

Added new unit tests for the various combinations, which pass.

Description From Last Updated

Can you keep the original (without :pserver:) as another example?

daviddavid
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/scmtools/cvs.py
        reviewboard/scmtools/tests.py
    
    Ignored Files:
        docs/manual/admin/configuration/repositories.rst
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/scmtools/cvs.py
        reviewboard/scmtools/tests.py
    
    Ignored Files:
        docs/manual/admin/configuration/repositories.rst
    
    
  2. 
      
david
  1. 
      
  2. Show all issues

    Can you keep the original (without :pserver:) as another example?

    1. I'd like to phase that out for a few reasons:

      1. It's more natural to use :pserver: if you're working with CVS, than to use our custom syntax, which was really just a workaround.
      2. That format prevents path-based matching.
      3. A method without :pserver: actually implies :ext: to CVS. If someone attempted to use that syntax with RB in order to get :ext: behavior, they'd have problems. (We'll never be able to fix this entirely, but I don't want to add confusion in docs).
    2. Makes sense.

  3. 
      
david
  1. Ship It!

  2. 
      
chipx86
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-2.0.x (15f66fe)
Loading...