-
Notifications
You must be signed in to change notification settings - Fork 393
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 GlobalConfiguration extension point to store all global config #68
Conversation
Thank you for a pull request! Please check this document for how the Jenkins project handles pull requests |
72fd2fa
to
3ca8885
Compare
@@ -0,0 +1,2 @@ | |||
<?xml version='1.0' encoding='UTF-8'?> | |||
<com.cloudbees.jenkins.GitHubPushTrigger_-DescriptorImpl plugin="github@1.13.0-SNAPSHOT"/> |
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.
Should not be a SNAPSHOT. 1.12?
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.
it can be any. It's just current build version. (its not affect tests)
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.
Please use the released version. Otherwise it may be hard to triangulate migration causes, etc.
this simplifies resulting config classes and xml-files
👍 |
public void setConfigs(List<GitHubServerConfig> configs) { | ||
this.configs = configs; | ||
} | ||
|
||
public List<GitHubServerConfig> getConfigs() { | ||
return configs; |
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.
FindBugs will complain about the internal implementation exposure
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.
will think about it on find bugs stage
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.
agreed
👍 |
use GlobalConfiguration extension point to store all global config
this simplifies resulting config classes and xml-files
new config looks like
(no migration from #65)
inspired by jenkinsci/envinject-plugin#57