[JENKINS-24702] Migration to credentials instead of plain token usage #65
Conversation
| /** | ||
| * @author lanwen (Merkushev Kirill) | ||
| */ | ||
| @XStreamAlias("github") |
There was a problem hiding this comment.
to simplify xml (for easier migration)
<github-plugin plugin="github@1.12.1-SNAPSHOT">
<configuration>
<configs>
<github>
<credentialsId>0986964f-727c-4684-8a01-91bbf1238bd2</credentialsId>
</github>
<github>
<apiUrl>https://gh.ent.company.com/api/v3</apiUrl>
<credentialsId>0986964f-727c-4684-8a01-91bbf1238bd2</credentialsId>
</github>
</configs>
</configuration>
</github-plugin>
|
Thank you for a pull request! Please check this document for how the Jenkins project handles pull requests |
lanwen
changed the title
| private static final String UNKNOWN_TOKEN = "unkn"; | ||
|
|
||
| private String apiUrl; | ||
| private boolean dontUseItToMangeHooks; |
There was a problem hiding this comment.
bad name!
just call manageHooks = true
|
src/main/java/com/cloudbees/jenkins/GitHubRepositoryName.java, line 71 [r2] (raw file): Comments from the review on Reviewable.io |
| @@ -0,0 +1,22 @@ | |||
| <?xml version='1.0' encoding='UTF-8'?> | |||
| <com.cloudbees.jenkins.GitHubPushTrigger_-DescriptorImpl plugin="github@1.11.4-SNAPSHOT"> | |||
| <manageHook>true</manageHook> | |||
There was a problem hiding this comment.
its old config. manageHook
https://github.com/jenkinsci/github-plugin/pull/65/files#diff-345d07212317de634c25ffbdc81f614aL217
|
src/main/java/com/cloudbees/jenkins/GitHubRepositoryName.java, line 90 [r2] (raw file): Comments from the review on Reviewable.io |
|
src/main/java/org/jenkinsci/plugins/github/config/GitHubServerConfig.java, line 63 [r1] (raw file): Comments from the review on Reviewable.io |
|
src/main/java/com/cloudbees/jenkins/GitHubRepositoryName.java, line 90 [r2] (raw file): Comments from the review on Reviewable.io |
|
src/main/java/com/cloudbees/jenkins/GitHubRepositoryName.java, line 90 [r2] (raw file): Comments from the review on Reviewable.io |
|
Reviewable doesn't reply into line comments :( |
|
pom.xml, line 79 [r4] (raw file): Comments from the review on Reviewable.io |
|
sorry bad cache, deleted wrong comments. |
| } | ||
| } | ||
|
|
||
| f.entry(title: _("Additional actions"), help: descriptor.getHelpFile('additional')) { |
There was a problem hiding this comment.
Not obvious for end-user.
If it only helper for "generating token", then put it under f.advanced(title="Token generator") {}
There was a problem hiding this comment.
Not often case. Who wants, finds this without problem
|
Sent simplifcation in lanwen#1 Also it unclear what type of creds should be added because "Add" button doesn't limit possible key types (had reports in other plugin). Should GitHub have special credentials type to have "Github Token" type to not mix it with other possible types? |
|
@KostyaSha I desagree with such simplification. It's not a simplification it's a just another ui with one more textbox, showed all time. |
|
Any credentials selection use case improvements should be done in separate prs. I don't plan to rewrite all use cases here |
I didn't mean you implement it here. Just noted.
KostyaSha@ab0f56a imho this a real bugfix, see commit description |
instead of own plain token usage
- with credentials usage - with migrator from old gh-push-trigger descriptor config options to new gh-plugin - with bean class to store deprecated creds (used only for migration)
+ reformat some code + add getters for fields
cleanup all gh-push-trigger descriptor related lines
- for migration process - for changes in hooks - for configuration logic
- change dontManageHooks=false to manageHooks=true
…nced also fix some phrases in javadocs and help
[JENKINS-24702] Migration to credentials instead of plain token usage
JENKINS-24702
Continue of #45
Default empty configuration
With one credential (not saved)
With one credential (after refresh)
Advanced
Generated xml config: