Enhanced project keys #60
Conversation
|
Thank you for a pull request! Please check this document for how the Jenkins project handles pull requests |
|
Hey @scaytrase - thanks a bunch for your PR! Works great here. I'll review the code and merge later today. |
|
@gruetter Ok, tell me if I can do something more here |
| } catch (IOException e) { | ||
| logger.println("Unable to expand commit SHA value"); | ||
| e.printStackTrace(logger); | ||
| return null; |
There was a problem hiding this comment.
if you return null here, you would set null as the value of the JSON key key in line 662 above, which doesn't check for a null value returned by this method. Instead, I propose to log an error message like you do but instead use the project name as a default key.
There was a problem hiding this comment.
I also see some forgotten copy-paste by me from here. I'll fix that.
|
Finished the review, @scaytrase . Besides the |
|
Thanks, @gruetter I'll try to fix all today or at least tomorrow morning |
|
well, cloudbees automerge now fails. should I rebase or something like? |
| } | ||
| } | ||
|
|
||
| String overriddenKey = (projectKey != null && projectKey.trim().length() > 0) ? projectKey : getDescriptor().getProjectKey(); |
There was a problem hiding this comment.
I'm not sure it is the best way to check that parameter is not empty. Is this correct? projectKey is never set up at global configuration, so I can miss something due to I'm new to Jenkins API
There was a problem hiding this comment.
I'm not much of a Jenkins pro either but your code looks good to me. I used the same approach and it has worked just fine in the past.
* Option to override project key for a certain project * Option to prepend project key with parent project
|
@gruetter Rebased. I've updated logic to prepend parent key (if checked) even if override key logic selected. After your review I thought that this makes more sense. |
|
Looks all good to me know. I'll merge. Thanks for following up @scaytrase . Great contribution. |
|
Thank you for good plugin we can contribute to! 👍 |
…ct-keys Enhanced project keys
Here is implementation for two options
All changes should be BC as disabled on empty (by default). All changes built and tested on my own jenkins installation.