-
-
Notifications
You must be signed in to change notification settings - Fork 9k
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
add missing @Symbol for agent protocols #2960
Conversation
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.
🐝 and CC @reviewbybees
@ndeloof do you consider it as a defect, which needs backporting to 2.60.3 or to 2.whatever.1? If yes, please create a JIRA ticket
@@ -307,6 +308,7 @@ private void error(PrintWriter out, String msg) throws IOException { | |||
* @since 1.653 | |||
*/ | |||
@Extension | |||
@Symbol("ping") |
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.
I doubt there is a use-case for it, but OK
@ndeloof It would be also great if you start following the PR template as @daniel-beck suggested in your previous pull requests. It saves time for the PR integration. |
definitively not a defect to be backported, I don't know where such @symbol is used for now, just noticed it was not used homogeneously, so this PR. |
Added the changelog entry |
What's the point of this, given the state https://github.com/jenkinsci/system-config-dsl-plugin is in? Does anyone care about this? Job DSL perhaps? Updated to add: Missed previous comments explaining this is only for consistency. Sure. Whatever. Not worth a changelog entry as it affects nobody, similar to Javadoc updates. |
@daniel-beck JNLP/JNLP2 are already annotated, hence it is just a consistency change. It may affect configuration-as-code use-cases, so I vote for changelogging it as RFE |
@daniel-beck this is not for system-config-dsl-plugin but some research project I'm working on, I just noticed those components where missing @symbol while others do have one, wanted to make this minor improvement for homogeneous API, nothing much. |
system-config-dsl isn't anywhere near done, does anyone actually use that? And nothing else uses symbols I'm aware of. Unless you know another (public) project, it's too irrelevant to mention. |
@reviewbybees done |
This pull request originates from a CloudBees employee. At CloudBees, we require that all pull requests be reviewed by other CloudBees employees before we seek to have the change accepted. If you want to learn more about our process please see this explanation. |
I just noticed JNLP3/4 and Ping agent protocols don't have
@Symbol
annotation, while JNLP1/2 have onChangelog Entry
@Symbol
annotations to JNLP4-connect, JNLP3-connect and Ping Remoting protocols