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

Inline jenkins-stapler-support #10189

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

basil
Copy link
Member

@basil basil commented Jan 21, 2025

A handful of annotation classes live in a separate jenkins-stapler-support library at https://github.com/jenkinsci/lib-jenkins-stapler-support, which hasn't been maintained in 6 years. I can't imagine anything other than Jenkins core consuming this library, and having it in a separate repository adds to the burden of maintaining it, so why not inline it into Jenkins core as we did with extras-executable-war a few years back. Its tests live in this repository anyway, so this unites the code with its tests, which is also a good thing. If this PR is accepted, I will archive the jenkins-stapler-support repository once it is no longer consumed by any LTS release.

Testing done

mvn clean verify -Dtest=jenkins.security.stapler.CustomRoutingDecisionProviderTest,jenkins.security.stapler.DoActionFilterTest,jenkins.security.stapler.DynamicTest,jenkins.security.stapler.GetterMethodFilterTest,jenkins.security.stapler.JenkinsSupportAnnotationsTest,jenkins.security.stapler.PreventRoutingTest,jenkins.security.stapler.Security400Test,jenkins.security.stapler.Security867Test,jenkins.security.stapler.Security914Test,jenkins.security.stapler.StaplerAbstractTest,jenkins.security.stapler.StaplerDispatchValidatorTest,jenkins.security.stapler.StaplerRoutableActionTest,jenkins.security.stapler.StaplerRoutableFieldTest,jenkins.security.stapler.StaplerRoutableGetterTest,jenkins.security.stapler.StaplerSignaturesTest,jenkins.security.stapler.StaticRoutingDecisionProvider2Test,jenkins.security.stapler.StaticRoutingDecisionProviderTest,jenkins.security.stapler.TypedFilterTest

Proposed changelog entries

N/A

Proposed upgrade guidelines

N/A

Submitter checklist

Preview Give feedback

Desired reviewers

@mention

Before the changes are marked as ready-for-merge:

Maintainer checklist

Preview Give feedback

@basil basil added the skip-changelog Should not be shown in the changelog label Jan 21, 2025
@basil basil requested a review from a team January 21, 2025 19:45
@basil basil added the needs-security-review Awaiting review by a security team member label Jan 21, 2025
@daniel-beck daniel-beck added security-approved @jenkinsci/core-security-review reviewed this PR for security issues and removed needs-security-review Awaiting review by a security team member labels Jan 21, 2025
@daniel-beck
Copy link
Member

IIRC, motivation at the time was to make the annotations available to plugins with an older core dependency. This is no longer relevant for these annotations. If we need something similar again, we can always create a new library with new annotations.

@krisstern
Copy link
Member

/label ready-for-merge

This PR is now ready for merge, after ~24 hours, we will merge it if there's no negative feedback.

@comment-ops-bot comment-ops-bot bot added the ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback label Jan 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback security-approved @jenkinsci/core-security-review reviewed this PR for security issues skip-changelog Should not be shown in the changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants