Fix port splitting logic to accomodate ssl: repositories for perforce.

Review Request #3719 — Created Jan. 5, 2013 and submitted

Information

RBTools
master

Reviewers

Fix port splitting logic to accomodate ssl: repositories for perforce.

If the perforce server uses ssl, there will be two colons in the P4PORT path
specifier. This would break our port-splitting logic.

 
Description From Last Updated

Maybe add a limit to the split, in case for some reason they put in another ":"? I know it …

chipx86chipx86

Blank lines around this.

chipx86chipx86

Col: 29 E127 continuation line over-indented for visual indent

reviewbotreviewbot

We repeat this twice. Maybe instead, have an is_valid flag, and check it after our if/elif/else, and die if set.

chipx86chipx86

Blank line.

chipx86chipx86
chipx86
  1. 
      
  2. rbtools/clients/perforce.py (Diff revision 1)
     
     
    Maybe add a limit to the split, in case for some reason they put in another ":"? I know it was broken before, but it's a good time to fix it.
  3. rbtools/clients/perforce.py (Diff revision 1)
     
     
     
     
     
     
    Blank lines around this.
  4. 
      
david
reviewbot
  1. This is a review from Review Bot.
      Tool: PEP8 Style Checker
      Processed Files:
        rbtools/clients/perforce.py
      Ignored Files:
    
    
  2. rbtools/clients/perforce.py (Diff revision 2)
     
     
    Col: 29
     E127 continuation line over-indented for visual indent
    
  3. 
      
david
reviewbot
  1. This is a review from Review Bot.
      Tool: PEP8 Style Checker
      Processed Files:
        rbtools/clients/perforce.py
      Ignored Files:
    
    
  2. 
      
chipx86
  1. 
      
  2. rbtools/clients/perforce.py (Diff revision 3)
     
     
     
    We repeat this twice. Maybe instead, have an is_valid flag, and check it after our if/elif/else, and die if set.
  3. rbtools/clients/perforce.py (Diff revision 3)
     
     
     
    Blank line.
  4. 
      
david
reviewbot
  1. This is a review from Review Bot.
      Tool: PEP8 Style Checker
      Processed Files:
        rbtools/clients/perforce.py
      Ignored Files:
    
    
  2. 
      
SM
  1. 
      
  2. rbtools/clients/perforce.py (Diff revision 4)
     
     
     
     
     
     
     
     
     
     
     
     
     
     
    Personally I'd prefer something along the lines of:
    
        parts = repository_path.split(':')
        hostname = None
    
        if len(parts) == 3 and parts[0] == 'ssl':
            hostname, port = parts[1:]
        elif len(parts) == 2:
            hostname, port = parts
    
        if hostname is None
            die('Path %s is not a valid Perforce P4PORT' % repository_path
    
    
    I'm fine with the way it is though, so you can make the call.
    1. Sure, that seems nicer. I'll do that before I push. Thanks!
  3. 
      
david
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to master (01385f8).
Loading...