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

[JENKINS-43733] Pick up the latest release of version-number library #2851

Merged
merged 2 commits into from Apr 29, 2017

Conversation

6 participants
@recena
Copy link
Contributor

commented Apr 20, 2017

Description

See JENKINS-43733.

Details: The main motivation is to keep this library updated on Jenkins core.

Changes between version-number-1.3 and version-number-1.4: jenkinsci/lib-version-number#3

Changelog entries

Proposed changelog entries:

  • N/A

Submitter checklist

  • JIRA issue is well described
  • Link to JIRA ticket in description, if appropriate

Desired reviewers

@jenkinsci/code-reviewers
@reviewbybees

@recena recena force-pushed the recena:version-number branch from c769544 to 9f3857a Apr 20, 2017

@oleg-nenashev

This comment has been minimized.

Copy link
Member

commented Apr 20, 2017

No need in the placeholder JIRA if it is an improvement without backporting need. 🐝 anyway

@reviewbybees

This comment has been minimized.

Copy link

commented Apr 20, 2017

This pull request originates from a CloudBees employee. At CloudBees, we require that all pull requests be reviewed by other CloudBees employees before we seek to have the change accepted. If you want to learn more about our process please see this explanation.

@jglick
Copy link
Member

left a comment

You should fix OldDataMonitor.VersionRange.isOld now that there is a replacement, as mentioned in jenkinsci/lib-version-number#3.

@recena

This comment has been minimized.

Copy link
Contributor Author

commented Apr 21, 2017

@jglick Addressed.

@recena recena closed this Apr 22, 2017

@recena recena reopened this Apr 22, 2017

@recena

This comment has been minimized.

Copy link
Contributor Author

commented Apr 22, 2017

The PR builder is failing due to something unrelated with these changes:

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:2.19.1:test (default-test) on project test: ExecutionException The forked VM terminated without properly saying goodbye. VM crash or System.exit called?
[ERROR] Command was cmd.exe /X /C "C:\Packages\Plugins\Microsoft.Compute.CustomScriptExtension\1.8\Downloads\0\tools\hudson.model.JDK\jdk8\jre\bin\java -Dfile.encoding=UTF-8 -Xmx1g -jar C:\Packages\Plugins\Microsoft.Compute.CustomScriptExtension\1.8\Downloads\0\workspace\Core_jenkins_PR-2851-7NNP5F22RZD2R5X4EAH2CSUY2YJI56S3FC4YDZ2LVB3IL262SFSA\test\target\surefire\surefirebooter156011780002022759.jar C:\Packages\Plugins\Microsoft.Compute.CustomScriptExtension\1.8\Downloads\0\workspace\Core_jenkins_PR-2851-7NNP5F22RZD2R5X4EAH2CSUY2YJI56S3FC4YDZ2LVB3IL262SFSA\test\target\surefire\surefire3610675378673748306tmp C:\Packages\Plugins\Microsoft.Compute.CustomScriptExtension\1.8\Downloads\0\workspace\Core_jenkins_PR-2851-7NNP5F22RZD2R5X4EAH2CSUY2YJI56S3FC4YDZ2LVB3IL262SFSA\test\target\surefire\surefire_58895678356336142184tmp"
[ERROR] -> [Help 1]

The tests were:

Tests run: 7877, Failures: 0, Errors: 0, Skipped: 5, Flakes: 1
@oleg-nenashev

This comment has been minimized.

Copy link
Member

commented Apr 22, 2017

may be related if version-number bundles JNI stuff somehow, but I doubt about that

@recena

This comment has been minimized.

Copy link
Contributor Author

commented Apr 22, 2017

@oleg-nenashev I don't think so. I linked the changes between 1.3 and 1.4.

@daniel-beck daniel-beck self-assigned this Apr 23, 2017

@daniel-beck daniel-beck self-requested a review Apr 23, 2017

@daniel-beck
Copy link
Member

left a comment

Would prefer if this were held off a bit until jenkinsci/lib-version-number#4 has been considered.

@daniel-beck daniel-beck removed their assignment Apr 24, 2017

@recena

This comment has been minimized.

Copy link
Contributor Author

commented Apr 24, 2017

@daniel-beck What do you mean with has been considered? Tested? Verified? Checked?

@oleg-nenashev

This comment has been minimized.

Copy link
Member

commented Apr 24, 2017

@recena I suppose @daniel-beck wants to avoid API deprecation by just updating to 1.5 with his proposal. Since this PR has missed 2.56, it seems reasonable to me to do that in 2.58 on next Sunday

@recena

This comment has been minimized.

Copy link
Contributor Author

commented Apr 24, 2017

@oleg-nenashev ah ok. Thanks for your explanation.

@daniel-beck

This comment has been minimized.

Copy link
Member

commented Apr 24, 2017

@recena AFAIUI, the change to OldDataMonitor would be unnecessary if my PR is considered useful and merged into version-number. If it's not, we can just go ahead here.

@@ -1 +1,2 @@
!*.zip
/0/

This comment has been minimized.

Copy link
@jglick

jglick Apr 24, 2017

Member

Huh? Please revert.

This comment has been minimized.

Copy link
@recena

recena Apr 25, 2017

Author Contributor

@jglick When you run the tests this folder is created. I thought it would be interesting to ignore it.

This comment has been minimized.

Copy link
@jglick

jglick Apr 25, 2017

Member

Created by what? I have never seen such a file. I think something may be broken on your machine.

This comment has been minimized.

Copy link
@daniel-beck

daniel-beck Apr 26, 2017

Member

I've seen these directories as well.

This comment has been minimized.

Copy link
@jglick

jglick Apr 26, 2017

Member

Containing what?

This comment has been minimized.

Copy link
@recena

recena Apr 26, 2017

Author Contributor

mvn test -Dtest=OldDataMonitorTest

output:

ip-192-168-1-111:test recena$ cd 0/
ip-192-168-1-111:0 recena$ ls -la
total 8
drwxr-xr-x  3 recena  staff  102 26 abr 18:31 .
drwxr-xr-x  8 recena  staff  272 26 abr 18:31 ..
-rw-r--r--  1 recena  staff  197 26 abr 18:31 build.xml
ip-192-168-1-111:0 recena$ 

This is being generated and hence, it should be ignored.

This comment has been minimized.

Copy link
@olivergondza

olivergondza Apr 26, 2017

Member

I would say it should be generated somewhere where it gets cleaned ...

This comment has been minimized.

Copy link
@jglick

jglick Apr 26, 2017

Member

Nope, fixing in #2860, so please revert.

This comment has been minimized.

Copy link
@recena

recena Apr 26, 2017

Author Contributor

@jglick thanks.

@recena recena force-pushed the recena:version-number branch from 5c9ec0f to 478bf9f Apr 26, 2017

@jglick

jglick approved these changes Apr 26, 2017

@oleg-nenashev
Copy link
Member

left a comment

lgtm

@oleg-nenashev

This comment has been minimized.

Copy link
Member

commented Apr 27, 2017

@oleg-nenashev

This comment has been minimized.

Copy link
Member

commented Apr 28, 2017

Will merge tomorrow if there is no feedback

@oleg-nenashev oleg-nenashev merged commit a32163f into jenkinsci:master Apr 29, 2017

2 checks passed

Jenkins This pull request looks good
Details
continuous-integration/jenkins/pr-head This commit looks good
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.