Update path(s) tasks, add tests, extract pluralize to strutil

Review Request #2892 — Created Sept. 25, 2015 and submitted

nhoward_tw
pants
2274
6f6e297...
pants-reviews
benjyw, jsirois, stuhood, zundel
The path / paths tasks were untested, used older idioms and had behavior that didn't reflect their documentation.

This updates them so that the print out the paths rather than just have a count.
It switches to using the console_output method instead of execute
It updates the algorithm used to cover cases more effectively, even in the face of cycles.

It also extracts pluralize from reporting and moves it to strutil and adds tests for it.

Wrote test suite, which uncovered weird behavior in the existing impl. Made it pass.

BE
  1. 
      
  2. Makes me wonder if we need this at all. Worth asking on pants-devel if we can deprecate this one and leave ./pants paths ?

    1. One reason for keeping it would be to prevent getting a huge dump of paths if you are iteratively removing a dependency. One thought I had was to change paths output s.t. it printed the count at the end. With that change, it could spool out the paths as it goes instead of gathering them into a list.

  3. 
      
JS
  1. 
      
  2. src/python/pants/util/strutil.py (Diff revision 1)
     
     
    Something like https://pypi.python.org/pypi/inflect may be in our future.  It would be nice polish to many of our messages.
    1. I was thinking something similar, but I wanted to push the ad hoc pluralizing into one place as a first step.

  3. I'd be very happy with symmetry in a follow-up RB.  Since this thing is broken before this RB, its ~proof its ok to change the CLI api.
    1. How do you think it should work? Maybe try from -> to, then to -> from?

    2. Yes.
  4. 
      
ST
  1. 
      
  2. src/python/pants/backend/core/tasks/paths.py (Diff revision 1)
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     

    Should this be a helper in the build graph instead? I guess it is a bit of a catchall, but...

    1. There's a couple of things like this in places. I think it could make sense to collect them together at least. I could move this function to build_graph as a top level fn as an incremental step. How would you feel about that?

  3. 
      
NH
Review request changed

Status: Closed (submitted)

Change Summary:

https://github.com/pantsbuild/pants/commit/4d691ca627fdd8fb5cf83776796e29c33de8f629

NH
  1. Thanks folks, submitted at https://github.com/pantsbuild/pants/commit/4d691ca627fdd8fb5cf83776796e29c33de8f629

    Follow up tasks
    add to -> from path lookup
    move path lookup helper to build_graph
    * maybe deprecate 'path' since it's redundant

  2. 
      
Loading...