Allow the dependees goal and idea to respect the --spec_excludes option.

Review Request #1795 — Created Feb. 19, 2015 and submitted

Eric Ayers
pants
zundel/dependees-respects-spec-excludes
1129
df0d685...
pants-reviews
benjyw, nhoward_tw, patricklaw, tejal

BuildFile.scan_buildfiles() now accept relative paths for spec_excludes.
BuildFile.dependants() now accepts the spec_excludes parameter.
Wire in spec_excludes into the dependees and idea goals.

Added unit tests for BuildFile.dependants() BuildFile.scan_buildfiles() and the dependees task.

Ran in the Square repo and it unlocks the dependees goal for us.

Eric Ayers
Benjy Weinberger
  1. I think this is OK, but I don't love repeatedly having to remember to pass spec_excludes into scan_buildfiles() everywhere.

    Could we have scan_buildfiles() default to the global property if the argument isn't explicitly specified? As far as I can tell we have no cases where it wouldn't want to use that anyway. Possibly ditto for the build root? That might be a bigger change though so maybe not yet.

    1. OK, that was just my comment that I didn't publish. Is there a global way to get at the global options object? I didn't see it.

    2. No,t yet but we could pluck that specific option out early on and set it as a class property of BuildFile? (and crash if it's not set yet, to ensure correctness?) Just one idea.

      We have this problem in several places - it's on my list of TODOs for the options system, to find a way to make options accessible outside of tasks.

    3. That kind of behavior would make testing miserable, don't you think?

    4. It's not ideal, for sure...

    5. I don't really like that idea. Any other paths you can see from here to Ship It? Want me to implement

      @classmethod
      def set_global_options(cls):
        ...
      
      @classmethod
      def get_global_options(cls):
        ...
      
  2. 
      
Eric Ayers
  1. What is making me excited about this change is that we have users that want to be able to refactor libraries across the repo. This will allow them to do:

    ./pants idea target:: $(./pants dependees target:lib)
    

    Is there another way to do this? I may follow on with a change to goal idea that will pull in dependees automatically.

    Of course, this is functionality I'd eventually like to see in the IntelliJ plugin.

  2. Having to thread this global option through several layers to finally get to BuildFile.scan_buildfiles() and BuildFile.dependents() is a bit of work and feels fragile. I almost wish we could access global options within BuildFile, but that would kind of pollute it as a library.

  3. 
      
Benjy Weinberger
  1. Oh, sorry, I thought I gave you a ship it already. The rest of it was just musing on my part.

  2. 
      
Eric Ayers
Review request changed

Status: Closed (submitted)

Change Summary:

Thanks Benjy! Commit 2fbf8ef

Patrick Lawson
  1. Ship It!
  2. 
      
Loading...