-
Notifications
You must be signed in to change notification settings - Fork 3
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
External Logging API: Align with new Core version, fix Findbugs #2
External Logging API: Align with new Core version, fix Findbugs #2
Conversation
It somehow picked the wrong core build... |
Yeah, it takes code from jenkinsci/jenkins#3575 (there were to deployments). I will bite the bullet and just refactor it, can be reverted later |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just merge it if that is helpful. I doubt anyone reviewed every line of the original anyway.
@@ -20,7 +20,9 @@ | |||
<properties> | |||
<revision>1.0-alpha-1</revision> | |||
<changelist>-SNAPSHOT</changelist> | |||
<jenkins.version>2.135-rc15088.42aa6febbbed</jenkins.version> | |||
<!--TODO: replace by incrementals (JENKINS-52873) --> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should not take more than a moment right?
@@ -15,6 +15,7 @@ | |||
final long timestamp; | |||
final long id; | |||
|
|||
//TODO:Consider <String,String> so that Key-Value storages could be quickly implemented |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes please; I do not think implementations can be expected to take arbitrary Java objects.
Agreed |
Hi @oleg-nenashev and @jglick, the Jenkins incrementals version cannot be resolved - see https://ci.jenkins.io/job/Plugins/job/external-logging-api-plugin/job/referenceImpl/. Can you tell what this version was about and why it is not available at https://repo.jenkins-ci.org/incrementals/org/jenkins-ci/main/jenkins-core/? |
See the referenced upstream PR in the description |
This is a new PR, which aligns compatibility with jenkinsci/jenkins#3575
LogStorage
ExternalLogBrowser
andExternalLoggingMethod
@reviewbybees @carlossg @jglick