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

Use new isValid() API for ParameterDefinition. #45

Merged
merged 5 commits into from
Dec 8, 2023

Conversation

jeffret-b
Copy link
Contributor

This demonstrates how to use the new ParameterDefinition.isValid method instead of implementing the logic locally. This is related to the new API in core from jenkinsci/jenkins#4826. See JENKINS-62889. This enhancement is expected to first appear in Jenkins 2.245 and this PR cannot be merged until that change is incorporated into all supported versions.

The test failure is related to JENKINS-62889.

@jglick jglick requested a review from dwnusbaum May 13, 2022 12:55
@jglick
Copy link
Member

jglick commented May 13, 2022

Somehow relates to #46 but I do not follow the context. @dwnusbaum do we still want this PR? Do we need some other sort of cleanup?

@jeffret-b jeffret-b requested a review from a team as a code owner December 8, 2023 21:14
@jglick
Copy link
Member

jglick commented Dec 8, 2023

@dwnusbaum do you recall what this was about, and whether it is still useful?

@dwnusbaum
Copy link
Member

I do not remember it. It seems to be cleanup to use a new API from jenkinsci/jenkins#4826 instead of duplicating the check logic.

Copy link
Member

@jglick jglick left a comment

Choose a reason for hiding this comment

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

🤷 the AbortException.message is not as nice. OTOH this could potentially handle other ParameterDefinition.isValid overrides. Seems there is still some tech debt related to #42, I guess because the API design for parameters has some fundamental flaws in terms of introspection.

@jglick jglick merged commit 8c88916 into jenkinsci:master Dec 8, 2023
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants