Move more taks to core_tasks.

Review Request #3199 — Created Dec. 1, 2015 and submitted

benjyw
pants
pants-reviews
jsirois, zundel

Changed a few names, and put each task in its own file,
for uniformity.

Also replaced the first=True argument to run_prep_command's
install with a comment explaining that it must be first in its
register.py, and another comment in extension_loader explaining
that core_tasks must be registered first.

The reason is that first=True was no less fragile - if anything
else registered into the 'test' goal with first=True later on,
it would preempt run_prep_command, with no comment. At least this way
the comments are clear, and we can maybe (??) move towards
deprecating first=True, or at least relying on it less.

CI passes: https://travis-ci.org/pantsbuild/pants/builds/94466648

BE
BE
ZU
  1. 
      
  2. src/python/pants/core_tasks/register.py (Diff revision 2)
     
     

    Why not ./src/python/pants/reporting/ ?

    1. We have about 85% adherence to an informal convention of one Task per source file, with the file named for the task, so I was trying to stick to that.

    2. What I mean is that we already have a package for reporting. Maybe the tasks to start and stop the reporting server should live there?

    3. Oh I see. I guess I preferred to concentrate all task code in core_tasks. Where you thinking the tasks should live there but be registered here? Or would reporting have its own register.py?

    4. 1) We could create another register.py

      or

      2) We could still register them in core_tasks/register.py, but the task code could live in reporting next to the reporting server.

    5. I'm reasonably strongly against 1).

      I could do 2) but I kind prefer to have the tasks live here, next to their friends. How strongly do you feel about this? I'm about 55%-45%, so easily persuadable.

    6. I don't feel very strongly, just wanted to know if you had considered this or maybe the possibility was overlooked.

    7. I hadn't really, but I considered it after you brought it up. :)

  3. src/python/pants/core_tasks/register.py (Diff revision 2)
     
     

    Why not ./src/python/pants/reporting/

  4. src/python/pants/core_tasks/register.py (Diff revision 2)
     
     

    Aside: elminating 'first=true' by registering first is onliy an option if you have the ability to edit the pants core source code. I have an alternative to the prep command that I install as a regular plugin. Obviously, it isn't the first task to register and there's not a way I can force this to happen as a plugin author. Someone can come in later and install another task as 'first' but I think that the intention of 'first' is still kept - it runs before the regular compile logic within the same step.

    I know this is one of the things I've heard might be addressed by the engine refactor, I don't think we should depreate 'first' until after that.

    1. Yeah, the more I look into it the more I realize that we can't get rid of this entirely. But I would like to reduce reliance on it, as it does still imply something that isn't strictly true, and which we're sort-of abusing in order to work around the deficiences of the current engine.

      I do believe that the new engine should get rid of this, since everything will be product-based.

  5. 
      
BE
ZU
  1. Ship It!
  2. 
      
BE
BE
  1. Thanks Eric! Submitted as d1294ff4113b4ccb0a858a41a3c97f654aeea5c1.

  2. 
      
BE
Review request changed

Status: Closed (submitted)

Change Summary:

d1294ff4113b4ccb0a858a41a3c97f654aeea5c1

Loading...