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.

Sign up for GitHub

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

Jump to bottom

[JENKINS-62880] Migrate RedisFingerprintStorage to use Descriptor Configuration #36

Merged
merged 26 commits into from
Jul 17, 2020
Merged

[JENKINS-62880] Migrate RedisFingerprintStorage to use Descriptor Configuration #36

merged 26 commits into from
Jul 17, 2020

Conversation

stellargo
Copy link
Contributor

No description provided.

@stellargo stellargo changed the title Move fingerprint configuration to use descriptors [JENKINS-62880] Migrate RedisFingerprintStorage to use Descriptor Configuration Jul 9, 2020
@stellargo stellargo added the WIP label Jul 9, 2020
@stellargo stellargo mentioned this pull request Jul 9, 2020
Merged
10 tasks
@lgtm-com
Copy link

lgtm-com bot commented Jul 10, 2020

This pull request introduces 1 alert when merging f41bfd5 into b5496b9 - view on LGTM.com

new alerts:

  • 1 for Double-checked locking is not thread-safe

@stellargo
Copy link
Contributor Author

@afalko @oleg-nenashev @mikecirioli this is the downstream PR to jenkinsci/jenkins#4834

This was referenced Jul 12, 2020
Merged
Merged
@stellargo stellargo closed this Jul 13, 2020
@stellargo stellargo reopened this Jul 13, 2020
@stellargo stellargo requested review from timja and afalko July 13, 2020 02:00
@stellargo stellargo marked this pull request as ready for review July 13, 2020 02:01
@stellargo stellargo requested a review from a team as a code owner July 13, 2020 02:01
@stellargo stellargo added feature and removed WIP labels Jul 13, 2020
@stellargo stellargo requested a review from oleg-nenashev July 13, 2020 17:59
timja
timja reviewed Jul 14, 2020
View reviewed changes
Jenkinsfile Outdated Show resolved Hide resolved
@stellargo stellargo closed this Jul 15, 2020
@stellargo stellargo reopened this Jul 15, 2020
oleg-nenashev
oleg-nenashev approved these changes Jul 15, 2020
View reviewed changes
Copy link
Member

@oleg-nenashev oleg-nenashev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. I have not tested it locally, but I do not see anything wrong. Some extra JCasC compatibility tests would be useful, but they can be added later

plugin/src/main/java/io/jenkins/plugins/redis/RedisFingerprintStorage.java Outdated Show resolved Hide resolved
plugin/src/main/java/io/jenkins/plugins/redis/RedisFingerprintStorage.java Outdated Show resolved Hide resolved
plugin/src/main/java/io/jenkins/plugins/redis/RedisFingerprintStorage.java Outdated Show resolved Hide resolved
plugin/src/main/java/io/jenkins/plugins/redis/RedisFingerprintStorageDescriptor.java Outdated Show resolved Hide resolved
@stellargo stellargo closed this Jul 15, 2020
@stellargo stellargo reopened this Jul 15, 2020
oleg-nenashev
oleg-nenashev approved these changes Jul 16, 2020
View reviewed changes
Copy link
Member

@oleg-nenashev oleg-nenashev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@stellargo
Copy link
Contributor Author

Will merge it in 24 hours if no major negative feedback.

@stellargo stellargo closed this Jul 17, 2020
@stellargo stellargo reopened this Jul 17, 2020
@stellargo stellargo merged commit 6a329e6 into jenkinsci:master Jul 17, 2020
@stellargo stellargo deleted the descr branch July 24, 2020 18:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@timja timja timja approved these changes

@oleg-nenashev oleg-nenashev oleg-nenashev approved these changes

@afalko afalko afalko approved these changes

Assignees
No one assigned
Labels
feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@stellargo @afalko @oleg-nenashev @timja