sample code "hello world" in python

Review Request #199 — Created April 10, 2014 and submitted

lahosken
pants
https://github.com/pantsbuild/pants/issues/24
pants-reviews
shows:
  python library
    (with 3rdparty dep)
  python binary
  python tests
Added a test. It passes. Added a reference to it in pants_test:all to make sure it keeps passing.
  • 0
  • 0
  • 4
  • 0
  • 4
Description From Last Updated
BE
  1. Thanks for adding this Larry. In future I encourage you to name a specific reviewer (as well as sending to pants-reviews), so you don't get hit by the bystander effect.
  2. We discourage code in __init__.py. There's no need for this here (or almost anywhere) so please remove.
    1. Hmm. Why so? If you have a dirname/file.py and like to use it in import as "from x.y.dirname import file" helps and makes it more explicit.
  3. src/python/example/hello/greet/greet.py (Diff revision 1)
     
     
    Let's add our __future__ imports to all the .py files here. 
    
    I know these are technically examples of other people's code, not Pants code, so they don't necessarily need to follow Pants code rules. However since people like to cut and paste, I'd rather these had those imports.
    1. Also, probably the same folks will end up maintaining the code; it makes sense for the same standards to apply to both.
  4. 
      
BE
  1. 
      
  2. A) Being able to import a module from multiple paths can cause accidental import cycles. This has happened to us more than once. These are hard to debug.
    
    B) This is unnecessary indirection sugar that just confuses the casual code reader.
  3. For uniformity with the rest of the codebase, please use double quotes in triple-quotes: """ not '''.
  4. 
      
LA
  1. Suggestions so far sound lovely. Asking a question here while I'm thinking of it:
    
    OH at a meeting: pants_test is called pants_test instead of pants to un-confuse Python debuggers.
    
    Should tests/python/example be tests/python/example_test instead?
    1. Possibly. pants_test is called pants_test not so much to un-confuse debuggers as because Python, unlike the JVM, doesn't like having logical packages (in our case pants.*) split across multiple filesystem locations (src/ vs tests/). You have to jump through various unsupported hoops to get that to work, and it's needlessly confusing.
      
      So yes, you're probably correct on this.
  2. 
      
LA
BE
  1. Ship It!
  2. 
      
LA
Review request changed

Status: Closed (submitted)

Loading...