Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Also support 1.x version schemes #67

Merged
merged 2 commits into from
Nov 27, 2018

Conversation

batmat
Copy link
Member

@batmat batmat commented Nov 26, 2018

Small followup on #63 and #64

Despite https://openjdk.java.net/jeps/223 officially deprecating 1.x schemes, we should be permissive here to avoid crashes.

For instance, in Jenkins the version in use is 1.${java.level}.
https://github.com/jenkinsci/plugin-pom/blob/master/pom.xml#L522

So supporting both formats will avoid unnecessary headaches around. We'll see possibly in the future to stop supporting this when the oldest line is >=9 and no more 1.x format can exist.

cc @jenkinsci/java11-support

Despite https://openjdk.java.net/jeps/223 officially deprecating 1.x
schemes, we should be permissive here to avoid crashes.

For instance, in Jenkins the version in use is `1.${java.level}`.
https://github.com/jenkinsci/plugin-pom/blob/master/pom.xml#L522

So supporting both formats will avoid unnecessary headaches around.
We'll see possibly in the future to stop supporting this when the oldest
line is >=9 and no more 1.x format can exist.
@oleg-nenashev
Copy link

No opinion. My preference would be to require all users to pass the version numbers correctly as documented by https://openjdk.java.net/jeps/223. But this patch simplifies the adoption OTOH

@batmat
Copy link
Member Author

batmat commented Nov 26, 2018

so you mean we should remove the 1. in https://github.com/jenkinsci/plugin-pom/blob/master/pom.xml#L522 @oleg-nenashev ? Fine by me too, though we need to test/check the potential impact there.

As the plugin has supported 1.x and x since 1.6, I think this should be fine. But I won't feel confident until we've checked :-)

@oleg-nenashev
Copy link

@batmat Yes, I am doing it in jenkinsci/plugin-pom#133

Given we use a LinkedHashMap, then we're guaranteed to find the first
inserted occurrence.
Not a big deal anyway since this is (and shouldn't in general) only cared
about the `renderVersion` related test.
@batmat batmat merged commit 989635c into mojohaus:master Nov 27, 2018
@batmat batmat deleted the also-support-1.x-formats branch November 27, 2018 08:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants