Add the scala 2.12 platform

Review Request #4388 — Created Nov. 18, 2016 and submitted

stuhood
pants
4068
pants-reviews
benjyw, mateor, molsen, wisechengyi
  • Add a platform for 2.12
  • Drop version-specificity for scalastyle, since it only inspects the sources
  • Float the version of scrooge-gen/scrooge-linter, because they do not mix classpaths with the things they are building/checking

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

  • 0
  • 0
  • 0
  • 2
  • 2
Description From Last Updated
BE
  1. I don't see anything relating to the scrooge changes you mentioned in the review description.

    Also, why RB and not github pull request? :)

    1. Ah, thanks. Updating now.

      Also, why RB and not github pull request? :)

      Indeed! Was planning to get that kicked off tomorrow. Sorry!

  2. Do we need this namedtuple any more? Can we just use strings directly now?

    1. Yes, probably. But I wasn't sure whether additional tool versions would be re-introduced at some point, so removing seemed premature.

    2. Fair enough.

  3. Is this always true? Wouldn't we need a minimum version of scalastyle that understands new language constructs if any?

    1. Yes, it is possible that new syntax (there hasn't been any in a long while) will require a new scalastyle version. But it is definitely not linked to the platform version currently, and it's not clear which platforms a new version will be available for... so IMO it's not worth attempting to guess the future there.

    2. Right, so in that case we'd have to model language versions, which are distinct from runtime versions. That's technically true of Java, although they always release the two in lockstep in practice.

  4. 
      
ST
BE
  1. Ship It!
  2. 
      
WI
  1. Ship It!
  2. 
      
MA
  1. 
      
  2. This seems like the right thing to ship, but it does hold the possibility of silently changing the jline version people are using. Seems like an unavoidable consequence of having hardcoded them in the first place, though

    I think it is okay to assume that only a pants-expert is likely to be using with_jline and therefore would be able find this patch and understand it as a correctness improvement.

    1. @mateo: The caller only ever passes 2.10 with with_jline, which is why this worked before.

    2. well, in Pants master. I override the repl and call this function outside of Pants code.

    3. ha, oops. Lies. This is an internal method, so I had to reimplemented it internally. As you can probably tell, I do not add jline to our custom repl. Apologies, carry on.

  3. 
      
ST
Review request changed

Status: Closed (submitted)

Change Summary:

Merged as 290016a5cebb6b88238ae0660c8185e51a1c7cd6

Loading...