search

kuleuven / jenkins-mattermost-plugin

Jenkins plugin for Mattermost
MIT License
24 stars 46 forks source link

Adds argument for additional "text" field of webhook payload #24

Closed charlietsai closed 7 years ago

charlietsai commented 7 years ago

In response to: https://github.com/jovandeginste/jenkins-mattermost-plugin/issues/5

Since it looks like attachments will not become highlighted or searchable in the meantime, I made this change.

This adds an optional text argument to mattermostSend that allows users to include formattable text. This can be used in Jenkinsfile for example.

charlietsai commented 7 years ago

mvn test :

...
-------------------------------------------------------
 T E S T S
-------------------------------------------------------
Running InjectedTest
Running jenkins.plugins.mattermost.MattermostNotifierTest
Running jenkins.plugins.mattermost.StandardMattermostServiceTest
Running jenkins.plugins.mattermost.workflow.MattermostSendStepIntegrationTest
Running jenkins.plugins.mattermost.workflow.MattermostSendStepTest
Tests run: 3, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.502 sec - in jenkins.plugins.mattermost.MattermostNotifierTest
Tests run: 13, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 1.229 sec - in jenkins.plugins.mattermost.StandardMattermostServiceTest
Tests run: 5, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 5.951 sec - in jenkins.plugins.mattermost.workflow.MattermostSendStepTest
Tests run: 7, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 24.525 sec - in InjectedTest
Tests run: 3, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 49.182 sec - in jenkins.plugins.mattermost.workflow.MattermostSendStepIntegrationTest

Results :

Tests run: 31, Failures: 0, Errors: 0, Skipped: 0
...
jovandeginste commented 7 years ago

@charlietsai Thanks a lot for your code, it looks perfect to me except for src/main/java/jenkins/plugins/mattermost/StandardMattermostService.java where the code was not indented. I know it's a nitpick, but would you mind amending an indentation-fix?

jovandeginste commented 7 years ago

@charlietsai more indentation ...

charlietsai commented 7 years ago

@jovandeginste: thanks for the quick reply, looks like I've used white space instead of tab-indents. Let me fix them all.

jovandeginste commented 7 years ago

@charlietsai Great, if all is well I'll merge and publish tomorrow!

charlietsai commented 7 years ago

@jovandeginste: all indentation should be fixed now. It looks like some files used white space while others used tabs, so I simply made sure that my changes were consistent within each file. This is just to reduce noise in the diff and since there is already an issue for code cleanup here.

jovandeginste commented 7 years ago

Released v2.4.1, coming to a Jenkins setup near you!

charlietsai commented 7 years ago

@jovandeginste thanks for the quick turnaround! I didn't update the Readme though, in case you wanted to do that.

jovandeginste commented 7 years ago

@charlietsai will do next time, unless you open a PR for that before me...

charlietsai commented 7 years ago

@jovandeginste: done 🙂 https://github.com/jovandeginste/jenkins-mattermost-plugin/pull/25