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

[JENKINS-52718] InstallState API should not be Restricted #3559

Merged
merged 1 commit into from Jul 28, 2018

Conversation

5 participants
@amuniz
Copy link
Member

commented Jul 24, 2018

See JENKINS-52718.

Proposed changelog entries

  • JENKINS-52718: Jenkins#{get,set}InstallState should not be @Restricted

Submitter checklist

  • JIRA issue is well described
  • Changelog entry appropriate for the audience affected by the change (users or developer, depending on the change). Examples
    * Use the Internal: prefix if the change has no user-visible impact (API, test frameworks, etc.)
  • Appropriate autotests or explanation to why this change has no tests
  • For dependency updates: links to external changelogs and, if possible, full diffs

Desired reviewers

@reviewbybees @jenkinsci/code-reviewers

@oleg-nenashev

This comment has been minimized.

Copy link
Member

commented Jul 25, 2018

No strong opinion, but it does not look like a good idea to manage it externally. Any specific use-case?

@amuniz

This comment has been minimized.

Copy link
Member Author

commented Jul 25, 2018

Well, it's being used in an core "internal" extension, so it seems to be a valid use of the API. Why an "external" one should have limited access?

We are using it in a non-public InstallState extension point implementation where we set the install state based on some conditions, so we need the API to get and set the state.

Also, if InstallState is an extension point, why the getter/setter of that field would be restricted?

@oleg-nenashev
Copy link
Member

left a comment

I think the use-case described by @amuniz is valid, +1 for permitting the API

@Wadeck

Wadeck approved these changes Jul 26, 2018

@oleg-nenashev oleg-nenashev merged commit 45bbec9 into jenkinsci:master Jul 28, 2018

2 checks passed

continuous-integration/jenkins/incrementals Deployed to Incrementals.
Details
continuous-integration/jenkins/pr-merge This commit looks good
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.