Refactor jar_library to be able to unwrap its list of jar_dependency objects

Review Request #1165 — Created Oct. 15, 2014 and submitted

zundel
pants
zundel/refactor-jar-library
680
7db5450...
pants-reviews
jsirois, patricklaw

This change is intended to support the external_archive() sharing the same logic as java_protobuf_library() for processing
a list of addresses to JarLibrary targets.

Added new unit test for new method in test_jvm_target
Also beefed up the test_jar_library unit test

CI is running

  • 0
  • 0
  • 2
  • 0
  • 2
Description From Last Updated
JS
  1. 
      
  2. unused - kill

  3. Please fixup the "import attribute" language here and below.

  4. This is the only "-style string in this file - you're making me twitch

  5. 
      
PA
  1. 
      
  2. This took me like four times to parse. It's correct, but maybe it could be reworded.

  3. type(spec).__name__

  4. If the upstream caller hasn't added library_specs to its traversable specs, this will just return None. Might want to add a bit of extra documentation noting this error mode.

  5. 
      
PA
  1. And one last general note: it's not clear to me that this should live on JarDependency, that feels like an abstraction leak. The latter doesn't know anything about the build graph or targets, it's just a plain old object. Maybe instead this should live on JvmTarget?

  2. 
      
ZU
PA
  1. Ship It!

  2. 
      
JS
  1. 
      
  2. kill blank line - these are in the same group

  3. The texwrap should go just under the boilerplate. I need to get a linter setup here soon:
    [future boilerplate]

    [stdlib]

    [3rdparty]

    [local]

  4. 
      
IT
  1. 
      
  2. it might be clearer to replace this with "if not isintance". seems like most of this method is handling when spec/target is not an instance of the expected type anyway.

    1. Good idea. I've made that change in the patch I'm working on now.

  3. 
      
ZU
Review request changed

Status: Closed (submitted)

Change Summary:

I actually submitted this last week.  Commit 2758b3c6c718ca687320ac6ae959239cda69ca07
Loading...