Refactor IvyTaskMixin's ivy_resolve and functions it depends on
Review Request #3371 - Created Jan. 26, 2016 and submitted
|Nick Howard (Twitter)|
|benjyw, gmalmquist, stuhood, zundel|
- Simplify and document IvyUtils.calculate_classpath
- Remove unnecessary
conf or ('default')s
- extract portion of
- Make cachepath return rather than yield. It already reads the full file, so it should have no memory impact.
- Rename cachepath to be more descriptive.
- Group variable usage with declarations in ivy task mixin
Ran unit tests. Successful travis at https://travis-ci.org/pantsbuild/pants/builds/104766229
Instead of calling
strip()twice you can do
filter(None, [path.strip() for path in ...])
Not that there's a performance issue with calling
strip()twice in this case, but I think this is neater, and more readable too.
s/objects/objects'/ (to clarify that multiple JarDependency objects are being modified)
And s/include/contain/ because 'include' is a loaded word when talking about excludes.
How about s/A tuple/A pair/ ?
Also my usual period nit.
Not introduced in your change, but since you're here:
s/targets up to date/targets are up to date/
and period... :)
incorporate Benjy's feedback.
Revision 2 (+101 -74)