Allow attaching a `prep_command` to several goals.

Review Request #4317 — Created Oct. 16, 2016 and submitted

jsirois
pants
jsirois/issues/3953
3953, 3971
0faf5c3...
pants-reviews
benjyw, cheister, zundel
Previously only a single goal could be specified leading to copy/paste
boilerplate targets and most likely a target aggregator when the
`prep_command` needed to be run in multiple goals with no common
ancestor. This change adds the `goals` kwarg to `prep_command` and
deprecates `goal` to allow reduction of boilerplate.

 src/python/pants/build_graph/prep_command.py                | 57 ++++++++++++++++++++++++++++++++++++--------------
 src/python/pants/core_tasks/run_prep_command.py             | 10 ++++-----
 tests/python/pants_test/core_tasks/test_run_prep_command.py | 40 ++++++++++++++++++++++++++++-------
 3 files changed, 78 insertions(+), 29 deletions(-)

Locally green:

./pants test \
  tests/python/pants_test/core_tasks:run_prep_command \
  tests/python/pants_test/core_tasks:prep_command_integration

CI went green here:
https://travis-ci.org/pantsbuild/pants/builds/167979392

BE
  1. Ship It!
  2. 
      
JS
ZU
  1. 
      
  2. It looks like you are trying to deprecate things, but changing this method name will break our repo because we have a custom prep command that adds to the run goal. Its easy for us to fix, but the "better way" from a compatibility standpoint would be to leave add_goal() and deprecate it and add the new method.

    1. So it sounds like you don't use RunPrepCommandBase?

    2. Yes we do. I think this call to add_goal() must have been a holdover from olden days. I commented it out and it still seems to work.

  3. 
      
ZU
  1. Ship It!
  2. 
      
JS
Review request changed

Status: Closed (submitted)

Change Summary:

Now on master:

git log -1 origin/master
commit 6c9d97adcf06d35c782a6c826e191e07228f9772
Author: John Sirois <john.sirois@gmail.com>
Date:   Mon Oct 17 10:02:28 2016 -0600

    Allow attaching a `prep_command` to several goals.
    
    Previously only a single goal could be specified leading to copy/paste
    boilerplate targets and most likely a target aggregator when the
    `prep_command` needed to be run in multiple goals with no common
    ancestor. This change adds the `goals` kwarg to `prep_command` and
    deprecates `goal` to allow reduction of boilerplate.
    
    Testing Done:
    Locally green:
    ```
    ./pants test \
      tests/python/pants_test/core_tasks:run_prep_command \
      tests/python/pants_test/core_tasks:prep_command_integration
    ```
    
    CI went green here:
      https://travis-ci.org/pantsbuild/pants/builds/167979392
    
    Bugs closed: 3953, 3971
    
    Reviewed at https://rbcommons.com/s/twitter/r/4317/
Loading...