DRY when handling Scala version values #20264
Conversation
alonsodomin
added
backend: JVM
| f"Scala cross version: {value}", | ||
| "Use value `binary` instead", |
There was a problem hiding this comment.
When do you expect this deprecation to hit? Is it user facing, or only plugin-author facing?
There was a problem hiding this comment.
It could be either way but the deprecation warning is mostly motivated to user facing, as users can define a cross-version mode using the crossversion field in the scala_artifact target. The motivation is down to the fact that in Scala-world the partial cross-version is, most of the time, dubbed as binary.
There was a problem hiding this comment.
Okay, thanks! Is there a more direct place you can put the deprecation? Such as in the relevant Field's constructor so that you can specify the self.address? This deprecation message will be hard for end-users to track down where the offending code is coming from.
There was a problem hiding this comment.
ok, I'll put in the target field
| if len(version_parts) != 3: | ||
| raise InvalidScalaVersion(scala_version) | ||
| return cls( | ||
| major=int(version_parts[0]), minor=int(version_parts[1]), patch=int(version_parts[2]) |
There was a problem hiding this comment.
I'm not very familiar with JVM versioning, but this would fail on any characters like rc1 or .alpha. Do you know if that's a risk?
There was a problem hiding this comment.
A quick scan of Maven repositories reveals that yeah, that could be a risk. At the end of the version is possible to have an arbitrary string separated by a - for non final versions.
I'll switch this to a regex match.
…n=partial (#20616) There were three deprecations originally scheduled for 2.21 (#20609), two of will have only been deprecated for one release, and aren't a significant burden to support, so we can ease users life for a little longer: | description | deprecation started in | what could be removed | |------------------------------------------------------|------------------------|-------------------------------------------------------------------------------------------------| | the `[GLOBAL].remote_oauth_bearer_token_path` option | 2.20.0.dev1 (#20116) | configuration for the option, plus some a few small `if` statements, all in `global_options.py` | | passing `crossversion="partial"` for scala artifacts | 2.20.0 (#20264) ^ | an extra enum variant, and a test | ^ NB. due to an easy-to-make misuse of `start_version` I believe people will only be getting the deprecation warnings in 2.20.0 stable, not any of the release candidates. The third (`python_awslambda`) is removed in #20619.
…n=partial (#20616) There were three deprecations originally scheduled for 2.21 (#20609), two of will have only been deprecated for one release, and aren't a significant burden to support, so we can ease users life for a little longer: | description | deprecation started in | what could be removed | |------------------------------------------------------|------------------------|-------------------------------------------------------------------------------------------------| | the `[GLOBAL].remote_oauth_bearer_token_path` option | 2.20.0.dev1 (#20116) | configuration for the option, plus some a few small `if` statements, all in `global_options.py` | | passing `crossversion="partial"` for scala artifacts | 2.20.0 (#20264) ^ | an extra enum variant, and a test | ^ NB. due to an easy-to-make misuse of `start_version` I believe people will only be getting the deprecation warnings in 2.20.0 stable, not any of the release candidates. The third (`python_awslambda`) is removed in #20619.
…n=partial (Cherry-pick of #20616) (#20623) There were three deprecations originally scheduled for 2.21 (#20609), two of will have only been deprecated for one release, and aren't a significant burden to support, so we can ease users life for a little longer: | description | deprecation started in | what could be removed | |------------------------------------------------------|------------------------|-------------------------------------------------------------------------------------------------| | the `[GLOBAL].remote_oauth_bearer_token_path` option | 2.20.0.dev1 (#20116) | configuration for the option, plus some a few small `if` statements, all in `global_options.py` | | passing `crossversion="partial"` for scala artifacts | 2.20.0 (#20264) ^ | an extra enum variant, and a test | ^ NB. due to an easy-to-make misuse of `start_version` I believe people will only be getting the deprecation warnings in 2.20.0 stable, not any of the release candidates The third (`python_awslambda`) is removed in #20619. Co-authored-by: Huon Wilson <[email protected]>
Noticing a lot of repetition when dealing with Scala version numbers and its particularities, like the binary compatibility version (required to figure out the actual JARs filenames), decided to make this small refactoring using a new
ScalaVersiondata class.