-
Notifications
You must be signed in to change notification settings - Fork 16
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-57078] pipeline integration #17
Conversation
@vtitov sorry, I do not longer maintain these repositories. |
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.
Looks like EnvInject APi already includes the desired implementations
*/ | ||
@Deprecated | ||
//@Restricted(NoExternalUse.class) // TODO re-enable deprecation & restriction | ||
public class EnvInjectActionPipelinedRetriever { |
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.
This class already has a Pipeline-compatible implementation here: https://github.com/jenkinsci/envinject-api-plugin/blob/master/src/main/java/org/jenkinsci/plugins/envinjectapi/util/EnvInjectActionRetriever.java
@Deprecated | ||
@Restricted(NoExternalUse.class) | ||
@SuppressFBWarnings(value = "SE_NO_SERIALVERSIONID", justification = "Deprecated code") | ||
public class EnvVarsPipelinedResolver implements Serializable { |
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.
https://github.com/jenkinsci/envinject-api-plugin/blob/master/src/main/java/org/jenkinsci/plugins/envinjectapi/util/EnvVarsResolver.java was refactored to support Pipeline. Looks like there is no practical differences between classes
Given @oleg-nenashev 's comments - do we need to worry about applying these changes at all, or simply consider envinject-lib mothballed and encourage the use of envinject-api instead? I might be willing to take this on if required in the near future if we feel there's still a need for development, but need the dust to settle on current work first. |
@TonyNoble all plugins should be using EnvInject API. That plugin still includes this library due to some compatibility reasons (mostly-XTrigger Lib), but I am inclined doing a breaking change once all dependent code is migrated |
Closing this pull request. Unless I am missing something, it is not needed |
No description provided.