-
Notifications
You must be signed in to change notification settings - Fork 411
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
Enhance Custom Message Functionality #169
Conversation
Thank you for this pull request! Please check this document for how the Jenkins project handles pull requests. |
👍 |
Was looking for this. Thanks! I'll test it asap. |
Any update on this? |
1 similar comment
Any update on this? |
Need at least two thumbs up if this is going to make it into 2.0 |
Well here's one thumb 👍 |
Needs to be updated to resolve conflicts. |
Seems to fix issue #163. |
I have updated this branch with the conflicts resolved. |
All I see is:
|
Unless I've missed something, this is merged now, according to Github - although it's pending the Jenkins validation at the moment. |
Would you mind rebasing to discard the merge commits? e.g. cd /path/to/your/slack-plugin
git checkout variable-custom-messages
git remote add upstream [email protected]:jenkinsci/slack-plugin.git
git fetch upstream
git rebase upstream/master
git push origin variable-custom-messages --force The rebase will auto-discard the merge commits. |
Done - sorry, my first time using git in anger :) |
Thanks. I'll let Kurt perform the merge. Definitely much cleaner. |
Any update on this? |
@kmadel ^^ |
@csogilvie Can you get this rebased off master again? It isn't mergeable right now. |
I will rebase, test and verify this again, however, this time I would like to see some action on it, rather than it just hanging around... as it is not a good way to encourage contributors :( |
This seems to check out OK from my testing. |
@csogilvie Thanks! I will try to get this tested out soon. Our Jenkin's looks like it needs to be updated in order to use the 2.x version of this plugin. Gotta get that updated now. |
I have rebased this again today as I noticed it had an issue. |
Please fix merge conflicts |
Done, again. |
This enhances the custom message functionality to include both a default custom message, if defined, and a specific custom message per build failure types.
I have squashed these and rebased it. |
Awesome I'll test/merge this evening. |
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.
Can't be merged as-is
After thorough testing. Upgrading the plugin with this version completely breaks all existing configurations in Jenkins. Until your change properly handles configuration migrations I can't approve merging it. If it were merged and released then it would effectively break any Jenkins installation with slack already configured when they go to upgrade.
Please address configuration migration.
Test yourself
$ mvn --version
Apache Maven 3.3.9 (bb52d8502b132ec0a5a3f4c09453c07478323dc5; 2015-11-10T08:41:47-08:00)
Maven home: /home/sam/usr/share/apache-maven-3.3.9
Java version: 1.7.0_79, vendor: Oracle Corporation
Java home: /home/sam/usr/share/java/jdk1.7.0_79/jre
Default locale: en_US, platform encoding: UTF-8
OS name: "linux", version: "4.4.0-64-generic", arch: "amd64", family: "unix"
-
Install Maven and ensure
mvn
is in your$PATH
. -
Bootstrap Jenkins with slack (it is already configured for https://jenkins-slack-plugin.slack.com/ ).
-
Run through a build sequence of jobs:
- Test (with parameters): fail checked, unstable unchecked
- Test: fail unchecked, unstable checked
- Test: fail unchecked, unstable unchecked
- Jervis
- Slack
-
Upgrade Jenkins to Slack plugin 2.2-SNAPSHOT built from merging your change into
master
and run through the build sequence of jobs in Step 3 of this list.
Slack output (before plugin upgrade)
- Slack 2.1 installed
Slack output (after plugin upgrade)
- Slack 2.2-SNAPSHOT installed with Enhance Custom Message Functionality #169 non-fast forward merge
@csogilvie I've posted my review. Let me know if you have any questions. As soon as a plugin upgrade cleanly upgrades (without affecting existing configurations), I will consider it good to merge. For how to configure migrations refer to past pull requests: https://github.com/jenkinsci/slack-plugin/pulls?q=is%3Apr+migrate+is%3Aclosed |
Is there any progress update on this, @csogilvie ? It's a feature that would be really great to have for our use cases. |
👍 |
1 similar comment
👍 |
@kmadel is there a way for regular users who haven't checked out to see what the conflict is? |
I will take a look once you merge changes from masters, looks like parts of this were already added/fixed by other merges. |
Closing this now to clean up the repo, |
Original idea from jenkinsci#169 (Enhance Custom Message Functionality by @csogilvie) Discussion in jenkinsci#385 I re-wrote it almost completely on top of the current master
This feature is a must for this plugin. Thanks for getting the ball rolling again KreAch3R |
Original idea from jenkinsci#169 (Enhance Custom Message Functionality by @csogilvie) Discussion in jenkinsci#385 I re-wrote it almost completely on top of the current master
Original idea from jenkinsci#169 (Enhance Custom Message Functionality by @csogilvie) Discussion in jenkinsci#385 I re-wrote it almost completely on top of the current master
Original idea from #169 (Enhance Custom Message Functionality by @csogilvie) Discussion in #385 I re-wrote it almost completely on top of the current master
This enhances the custom message functionality to include both a default custom message, if defined, and a specific custom message per build failure types.