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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restore jsr-305 dependency #2492

Merged
merged 1 commit into from
May 16, 2024
Merged

Restore jsr-305 dependency #2492

merged 1 commit into from
May 16, 2024

Conversation

timja
Copy link
Member

@timja timja commented May 16, 2024

Untested but I expect it to fix #2491

Your checklist for this pull request

馃毃 Please review the guidelines for contributing to this repository.

  • Make sure you are requesting to pull a topic/feature/bugfix branch (right side) and not your master branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or in Jenkins JIRA
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Did you provide a test-case? That demonstrates feature works or fixes the issue.

@timja timja requested a review from a team as a code owner May 16, 2024 15:55
@timja timja added the bug label May 16, 2024
@timja timja requested a review from MarkEWaite May 16, 2024 15:55
@timja timja enabled auto-merge (squash) May 16, 2024 15:55
Copy link
Contributor

@MarkEWaite MarkEWaite left a comment

Choose a reason for hiding this comment

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

I've confirmed this works with interactive testing in my lts-with-plugins container image.

Tests that I ran include:

  • Start Jenkins without it and confirm the message is displayed in the log
  • Explore Jenkins to see if there were obvious failures that a user would see (none detected)
  • Install into the lts-with-plugins plugins.txt file
  • Start Jenkins and confirm the message is not displayed
  • Display the configuration as code
  • Reload the configuration as code

@MarkEWaite
Copy link
Contributor

I was trying to add my proposed extra comment but I think it is wrong to delay the delivery of this fix. I'll add that comment later when I've investigated further.

@MarkEWaite
Copy link
Contributor

Notes that I put into my test commit included:

Fix init warning message by restoring JSR-305 dependency

#2490 removed the JSR-305 dependency because the code compiles and passes tests without that dependency. However, a new startup failure message is reported without that dependency. Retain the dependency for now until more investigation can identify the other changes needed to remove that dependency.

Thanks to @kutzi for reporting it promptly!

#2490 (comment) is the report.

Fixes #2491

@timja timja merged commit 0175eda into master May 16, 2024
15 of 17 checks passed
@timja timja deleted the timja-patch-2 branch May 16, 2024 16:26
@basil
Copy link
Member

basil commented May 16, 2024

History repeats itself one year later, almost to the day: #2238

@MarkEWaite
Copy link
Contributor

History repeats itself one year later, almost to the day: #2238

Yes, my mistake that I didn't perform the basic interactive test of loading it into my own configuration and watching the log file.

I'll submit a pull request to add a comment that warns others away from making the same mistake that I made.

MarkEWaite added a commit to MarkEWaite/configuration-as-code-plugin that referenced this pull request May 17, 2024
jenkinsci#2491
should be prevented from recurring in the future by this comment.

jenkinsci#2492 is
the pull request that restored the dependency that I mistakenly removed
in jenkinsci#2490
@MarkEWaite MarkEWaite mentioned this pull request May 17, 2024
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Nonnull dependency issue on startup
3 participants