Skip to content
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

Add display failed tests option to slack notifications on job configuration #275

Merged
merged 1 commit into from
Mar 3, 2017

Conversation

derinbay
Copy link
Contributor

I've added an option as "Include Failed Tests" to slack notifications on job configuration screen to display failed tests.

PR#166 also adds the failed tests but I think there should be an option for user to choose on configurations. Also there was a conflict on that PR and it was very old, so I've opened this one.
screen shot 2016-12-23 at 01 40 35

@samrocketman
Copy link
Member

Please rebase on master and resolve merge conflicts.

@samrocketman samrocketman self-requested a review March 2, 2017 07:14
@samrocketman samrocketman self-assigned this Mar 2, 2017
@derinbay
Copy link
Contributor Author

derinbay commented Mar 3, 2017

@samrocketman rebased and resolved conflicts.

@samrocketman
Copy link
Member

@derinbay if you've rebased, then why are there so many commits?

@derinbay
Copy link
Contributor Author

derinbay commented Mar 3, 2017

@samrocketman I guess I messed up with the rebase and made a merge from master instead, sorry about that.. Should I revert?

@samrocketman
Copy link
Member

@derinbay Do not revert. Cherry pick the merge commit onto a new branch based on upstream/master.

Note: I haven't tested the following method. I will test it tomorrow to be sure the commands are correct.

#from your own repository
git remote add upstream https://github.com/jenkinsci/slack-plugin
git fetch upstream
git checkout failed-tests-report
#rename the branch
git branch -m old-failed-tests-report
#create a new branch
git checkout upstream/master -b failed-tests-report
#get all of the changes as a single commit
git cherry-pick -m 2 old-failed-tests-report
#reuse metadata from your original commit
git commit --amend --reuse-message=1c86334fd262bd5a3caa82aeab6faab74948e2e9

Now you should be one commit ahead of upstream master.

#status should say you're one commit ahead of upstream/master
git status
#inspect your commit to ensure it is correct
git show HEAD

If everything looks okay to you then force push it to overwrite your branch. If you're unsure, then feel free to give me write access to your repository and I can fix it for you.

@derinbay derinbay force-pushed the failed-tests-report branch 2 times, most recently from 0cfdbd8 to a57ac02 Compare March 3, 2017 09:59
@derinbay
Copy link
Contributor Author

derinbay commented Mar 3, 2017

@samrocketman Made some stuff but not sure if it's ok. First it was fine but then I revert back my last commit, that commit resolves a duplication which is not exist anymore, probably it was happened during the merge yesterday, so it was unnecessary. I've added you as collaborator to my repo. I'll be glad if you check. Thanks

@samrocketman
Copy link
Member

samrocketman commented Mar 3, 2017

@derinbay I think you mistakenly messed up your branch. The PR shows 0 files changed. There's not much I can do in its current state. If you still have old-failed-tests-report then please push it up.

@derinbay derinbay force-pushed the failed-tests-report branch from 55b3d78 to 2cac73c Compare March 3, 2017 20:35
@derinbay
Copy link
Contributor Author

derinbay commented Mar 3, 2017

@samrocketman Ok I found the codes and saw that #166 was almost same development with this one. I've added the differences to this commit, that shows the duration of the failed test and fixed a lower case letter (I guess a typo)

@samrocketman
Copy link
Member

Looks good. I'm going to test it before merging. Thanks for your contribution.

@samrocketman samrocketman merged commit 364bca9 into jenkinsci:master Mar 3, 2017
@samrocketman samrocketman deleted the failed-tests-report branch March 3, 2017 22:20
@samrocketman samrocketman added this to the slack-2.2 milestone Mar 6, 2017
@samrocketman
Copy link
Member

Feel free to subscribe to #296 for updates related to Slack Plugin 2.2 release.

@snupsband
Copy link

Is it possible to send results in a pipeline using slackSend?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants