• 
      

    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?

    david david
    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:
    Completed
    Change Summary:
    Pushed to release-2.0.x (15f66fe)