• 
      

    New rb-config command - shows user configuration.

    Review Request #2548 — Created Aug. 14, 2011 and submitted

    Information

    RBTools

    Reviewers

    For now we can't provide command for changing '.reviewboardrc', so the new command only lists configuration values in pretty form.
    
     
    Description From Last Updated

    The "by default," wasn't addressed.

    chipx86chipx86

    No need for the \, since the parens guarantee Python won't get confused. "by default," (with the comma) and not …

    chipx86chipx86

    Seems like the set() math should be part of load_config_files instead. Once you've done that, you no longer need to …

    chipx86chipx86

    Can you import rbconfig instead, and then call rbconfig.main()?

    chipx86chipx86

    ouput?

    chipx86chipx86

    No need for re.split. You can just do str.split(delimiter). Also, we should avoid map and filter where possible. They're less …

    chipx86chipx86

    Should be clear that it's testing in rb config.

    chipx86chipx86

    Should be clear that it's testing in rb config.

    chipx86chipx86

    "Test the '-a/--all' option in rb-config"

    chipx86chipx86

    This needs to be: try: from cStringIO import StringIO except ImportError: from StringIO import StringIO

    chipx86chipx86

    I think you can use StringIO instead of this.

    chipx86chipx86

    ouput?

    chipx86chipx86

    I meant use StringIO here. We don't need OutputBuffer.

    chipx86chipx86

    Whatever is done here needs to be heavily commented. Can you instead just do: builtin = globals()?

    chipx86chipx86
    chipx86
    1. 
        
    2. rbtools/commands/rbconfig.py (Diff revision 1)
       
       
      Show all issues
      No need for the \, since the parens guarantee Python won't get confused.
      
      "by default," (with the comma) and not "by-default"
    3. rbtools/commands/rbconfig.py (Diff revision 1)
       
       
       
      Show all issues
      Seems like the set() math should be part of load_config_files instead.
      
      Once you've done that, you no longer need to convert this all into a dictionary. You'll just have one that you can iterate.
    4. rbtools/commands/tests.py (Diff revision 1)
       
       
      Show all issues
      Can you import rbconfig instead, and then call rbconfig.main()?
    5. rbtools/commands/tests.py (Diff revision 1)
       
       
      Show all issues
      ouput?
    6. rbtools/commands/tests.py (Diff revision 1)
       
       
       
       
       
       
      Show all issues
      No need for re.split. You can just do str.split(delimiter).
      
      Also, we should avoid map and filter where possible. They're less readable and Python suggests not using them. Instead, this can be:
      
      [s.strip() for s in line.split(":")]
      for line in text.splitlines() if line
      
      You also don't really need to turn this into a dictionary. This whole thing can be greatly simplified:
      
      values = [[s.strip() for s in line.split(":")]
                for line in text.splitlines() if line]
      
      for key, value in values:
          self.assertEquals(value, ...)
      
      Actually... What's this even doing? It's just comparing itself with itself. So something is wrong here.
      
    7. rbtools/commands/tests.py (Diff revision 1)
       
       
      Show all issues
      Should be clear that it's testing in rb config.
    8. rbtools/commands/tests.py (Diff revision 1)
       
       
      Show all issues
      Should be clear that it's testing in rb config.
    9. rbtools/utils/testbase.py (Diff revision 1)
       
       
       
       
       
       
       
       
       
       
       
      Show all issues
      I think you can use StringIO instead of this.
    10. rbtools/utils/testbase.py (Diff revision 1)
       
       
      Show all issues
      ouput?
    11. 
        
    MB
    chipx86
    1. 
        
    2. rbtools/commands/rbconfig.py (Diff revisions 1 - 2)
       
       
      Show all issues
      The "by default," wasn't addressed.
    3. rbtools/commands/tests.py (Diff revisions 1 - 2)
       
       
      Show all issues
      "Test the '-a/--all' option in rb-config"
    4. rbtools/utils/testbase.py (Diff revisions 1 - 2)
       
       
      Show all issues
      This needs to be:
      
      try:
          from cStringIO import StringIO
      except ImportError:
          from StringIO import StringIO
    5. rbtools/utils/testbase.py (Diff revisions 1 - 2)
       
       
      Show all issues
      I meant use StringIO here. We don't need OutputBuffer.
    6. rbtools/utils/filesystem.py (Diff revision 2)
       
       
      Show all issues
      Whatever is done here needs to be heavily commented.
      
      Can you instead just do: builtin = globals()?
      1. I think we discussed that on IRC. globals() values can intersect with those loaded from config and hence we will strike them out. exec gives as a 'pristine' copy of python environment. 
    7. 
        
    MB
    chipx86
    1. Looks good. Made a couple small grammatical fixes.
    2. 
        
    MB
    Review request changed
    Status:
    Completed
    Change Summary:
    Committed to api (4990e1d)