cleanup is_xxx checks: is_jar_library

Review Request #1719 — Created Feb. 5, 2015 and submitted

dturner-tw
pants
2c84dd9...
pants-reviews
ity, jsirois, patricklaw, zundel
Replace every instance of is_jar_library with isinstance check.  It's
a start.

https://travis-ci.org/pantsbuild/pants/builds/49652980

  • 0
  • 0
  • 0
  • 1
  • 1
Description From Last Updated
JS
  1. Ship It!
  2. 
      
ZU
  1. 
      
  2. src/python/pants/base/target.py (Diff revision 1)
     
     

    I am sorta OK with this and the removal of setting the label from jar_library . My reservations on removing them is that they are on target so could be used by anyone.

    In the other patch I started, I left these methods in place with a message on what to use instead. I thought about doing a print("DEPRECATED...") style message like we have used for other deprecated functions... so that you'd see the problem right away in your repo if you updated to the latest version of pants without it actually breaking.

    1. I'd sortof love to break people in this 0.0.X phase and move past deprecations quickly. I'm not adamant, but I'd certainly prefer moving on while we can.

    2. FWIW, I checked and Twitter does not use this. We do use is_java, is_scala, and is_jvm, but I could clean those up pretty quickly.

    3. oh, wait, we might rely on is_jar_library in one place. Whatevs I'll just refactor that too.

  3. 
      
DT
ZU
  1. I will add a deprecation message to is_jar_library() in https://rbcommons.com/s/twitter/r/1720/

  2. 
      
JS
  1. Ship It!
  2. 
      
DT
Review request changed

Status: Closed (submitted)

PA
  1. Just want to double check--none of these should have pulled in extra BUILD dependencies right?

  2. Imports aren't sorted (though you didn't introduce the original bad import)

    1. Yeah, we should do another big import fix, and then introduce a check for that so it doesn't happen again.

      Everything compiles/tests OK, so we're not directly missing anything; we could probably add a few more explicit deps rather than relying on transitive deps, but I think we already do rely on transitive deps a fair amount.

  3. 
      
Loading...