Add authentication files to pants bin target

Review Request #292 — Created April 29, 2014 and submitted

tejal
pants
90
pants-reviews
jsirois, lahosken, travis
Add authentication files to pants bin target
rm -rf pants.pex

./pants


./pants.pex goal list src/python/pants/authentication/BUILD
src/python/pants/authentication/BUILD:authentication
[tw-172-25-20-142 pants]$ 


New build url 
https://travis-ci.org/pantsbuild/pants/builds/24081581
  • 0
  • 0
  • 2
  • 0
  • 2
Description From Last Updated
JS
  1. 
      
  2. src/python/pants/authentication/BUILD (Diff revision 1)
     
     
    How about globs('*.py') (there is an __init__.py).
  3. src/python/pants/authentication/BUILD (Diff revision 1)
     
     
    This actually has a dep on TaskError, see: https://github.com/pantsbuild/pants/blob/master/src/python/pants/authentication/netrc_util.py#L11
    It probably should instead define its own nested error class and raise that - a downstream Task that actually rightfully knows about things like TaskError can trap and re-raise wrapping in one.
  4. src/python/pants/bin/BUILD (Diff revision 1)
     
     
    The comment and extra blank lines can go - there is nothing special about the afaict target except that it was left out one time.
  5. 
      
TE
TE
JS
  1. LGTM - a few small issues, 1 print to definitely clean up.
  2. Its on my plate to get a styleguide up, but I was going to start with Twitter's which says to nest this exception type in Netrc
  3. Please kill or use a logger
  4. 2 blank lines between imports and functions/classes: http://legacy.python.org/dev/peps/pep-0008/#blank-lines
  5. Unused - kill
  6. Since you patch os.path.expanduser so you need this environment patch in this test?
  7. Its a bit odd to extend unittest.TestCase and then do pytest style enhanced asserts.  Pick a style and either use unittest.TestCase and self.assertXXX or else just use bare top-level functions and assert py.test style.  SInce you have no real setUp you can go either route.
  8. this test and the next - could be DRYed up.  I can imagine a helper that let you write just:
    
    def test_netrc_parse_error(self):
      with netrc('machine white \n') as netrc:
        # test me
  9. 
      
TE
  1. 
      
  2. done
  3. 
      
TE
JS
  1. 
      
  2. s/Mack/Mock/ ?
  3. 
      
TE
Review request changed

Status: Closed (submitted)

Loading...