Clearcase Popen problem on buggy python 2.7.x

Review Request #3994 — Created March 23, 2013 and submitted

delyn
Review Board
reviewboard
moonese
Original issue was descibed in http://reviews.reviewboard.org/r/3804/
A console window would pop up every time popen is invoked on Windows 7 

Unfortunately, this fix has created a regression on other OSes
https://groups.google.com/forum/?fromgroups=#!topic/reviewboard-dev/hbrKN_p5Rvo

The goal of this update is to prevent Popen shell=true unless Windows 7 and python 2.7.x

Please comment it, do not whenever it is the best place/way to implement such _popen_shell=T/F

this can be merged from https://github.com/nanouck/reviewboard
on windows 7 and python 2.7.3
_popen_shell = True

on windows XP and python 2.7.3
_popen_shell = False
Description From Last Updated

Col: 1 W191 indentation contains tabs

reviewbotreviewbot

Col: 1 E101 indentation contains mixed spaces and tabs

reviewbotreviewbot

Col: 2 E128 continuation line under-indented for visual indent

reviewbotreviewbot

Col: 1 W191 indentation contains tabs

reviewbotreviewbot

Col: 1 E101 indentation contains mixed spaces and tabs

reviewbotreviewbot

Col: 1 W191 indentation contains tabs

reviewbotreviewbot

Col: 1 E101 indentation contains mixed spaces and tabs

reviewbotreviewbot

Please add reference to the bugs.

BA bartek

Col: 1 E128 continuation line under-indented for visual indent

reviewbotreviewbot

Col: 1 E128 continuation line under-indented for visual indent

reviewbotreviewbot

Should be proper sentences, with capitalization and periods.

chipx86chipx86

I know we don't support Python 3.x yet, but we will in time. Do we have any idea about what ...

chipx86chipx86

Is this a problem on Windows 8? Do we need to check for >= 7?

chipx86chipx86
reviewbot
  1. This is a review from Review Bot.
      Tool: PEP8 Style Checker
      Processed Files:
        reviewboard/scmtools/clearcase.py
      Ignored Files:
    
    
  2. reviewboard/scmtools/clearcase.py (Diff revision 1)
     
     
    Col: 1
     W191 indentation contains tabs
    
  3. reviewboard/scmtools/clearcase.py (Diff revision 1)
     
     
    Col: 1
     E101 indentation contains mixed spaces and tabs
    
  4. reviewboard/scmtools/clearcase.py (Diff revision 1)
     
     
    Col: 2
     E128 continuation line under-indented for visual indent
    
  5. reviewboard/scmtools/clearcase.py (Diff revision 1)
     
     
    Col: 1
     W191 indentation contains tabs
    
  6. reviewboard/scmtools/clearcase.py (Diff revision 1)
     
     
    Col: 1
     E101 indentation contains mixed spaces and tabs
    
  7. reviewboard/scmtools/clearcase.py (Diff revision 1)
     
     
    Col: 1
     W191 indentation contains tabs
    
  8. reviewboard/scmtools/clearcase.py (Diff revision 1)
     
     
    Col: 1
     E101 indentation contains mixed spaces and tabs
    
  9. 
      
BA
  1. 
      
  2. reviewboard/scmtools/clearcase.py (Diff revision 2)
     
     
     
    Please add reference to the bugs.
    1. Bart, I would have liked to, but I cannot found them in http://code.google.com/p/reviewboard/issues :-(
      You are the one who has reported regression, have you create such defect ?
    2. Oh, I thought about adding a similar explanation to the one in the review request with the link you mentioned: http://reviews.reviewboard.org/r/3804/ :)
    3. sorry, I missunderstood it :-(
  3. 
      
DE
reviewbot
  1. This is a review from Review Bot.
      Tool: PEP8 Style Checker
      Processed Files:
        reviewboard/scmtools/clearcase.py
      Ignored Files:
    
    
  2. reviewboard/scmtools/clearcase.py (Diff revision 3)
     
     
    Col: 1
     E128 continuation line under-indented for visual indent
    
  3. reviewboard/scmtools/clearcase.py (Diff revision 3)
     
     
    Col: 1
     E128 continuation line under-indented for visual indent
    
  4. 
      
DE
reviewbot
  1. This is a review from Review Bot.
      Tool: PEP8 Style Checker
      Processed Files:
        reviewboard/scmtools/clearcase.py
      Ignored Files:
    
    
  2. 
      
DE
chipx86
  1. 
      
  2. reviewboard/scmtools/clearcase.py (Diff revision 4)
     
     
     
     
    Should be proper sentences, with capitalization and periods.
  3. reviewboard/scmtools/clearcase.py (Diff revision 4)
     
     
    I know we don't support Python 3.x yet, but we will in time. Do we have any idea about what versions there we may need to factor in?
    1. the issue is known for python 2.7.x only at the moment, so I'd rather to keep this check but can add a comment to track this information.
  4. reviewboard/scmtools/clearcase.py (Diff revision 4)
     
     
    Is this a problem on Windows 8? Do we need to check for >= 7?
    1. currently we do not know if windows8 + python 2.7.x is affected, I have no windows 8 to check it. 
      
      IMHO ">=7" is better because python 2.7.x was released before windows 7 and 8 and don't believe such issue could have been fixed
    2. finally I've just updated the comment because platform.release() return a string and "7", "8", and other version should be checked but I cannot test these strings (no such OSes) so I'd rather keep current test like that until someone report same issue with another known value of platform.release()
  5. 
      
DE
reviewbot
  1. This is a review from Review Bot.
      Tool: PEP8 Style Checker
      Processed Files:
        reviewboard/scmtools/clearcase.py
      Ignored Files:
    
    
  2. 
      
BA
  1. Is there anything else that keeps this change from being submitted?
    1. Nope. We'll get it in for 1.7.10.
  2. 
      
DE
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-1.7.x (fbe1ad2)
Loading...