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

Enhance Custom Message Functionality (rebased from #169) #385

Closed
wants to merge 3 commits into from

Conversation

KreAch3R
Copy link
Contributor

The bd28d4e is a very valuable commit. It makes or breaks the slack-plugin experience for me. I took the time to merge it into the current master branch, and fix any problems that arose.

I have no idea how to fix the migration problems that #169 mentioned as the reason of no merge. In my experience, I just had to configure the project once and then save it. Afterwards, I could move the custom message where I wanted and it works.

I'm PRing this so that others can see and use it locally.

Colin Ogilvie and others added 3 commits February 28, 2017 11:05
This enhances the custom message functionality to include both a default custom message, if defined, and a specific custom message per build failure types.
@timja timja changed the title #169: merge into master Enhance Custom Message Functionality (rebased from #169) Nov 28, 2018
@timja
Copy link
Member

timja commented Nov 28, 2018

Needs conflicts fixing

@timja
Copy link
Member

timja commented Dec 4, 2018

feel free to reopen when you resolve conflicts

@timja timja closed this Dec 4, 2018
@KreAch3R
Copy link
Contributor Author

KreAch3R commented Dec 4, 2018

Hey @timja , thanks for taking over/updating this repo. I took a look at what changes are made so the original #169 commit can be merged in the current master.

It seems that with your commit: e1d919a you changed an important part of this commit, mainly the method newInstance, where this part was added:

HashMap<String,String> customMessageMap = new HashMap<String,String>();
customMessageMap.put("SUCCESS", sr.getParameter("customMessageBuildSuccess"));
customMessageMap.put("FAILURE", sr.getParameter("customMessageBuildFailure"));
customMessageMap.put("ABORTED", sr.getParameter("customMessageBuildAborted"));
customMessageMap.put("NOT_BUILT", sr.getParameter("customMessageBuildNotBuilt"));
customMessageMap.put("UNSTABLE", sr.getParameter("customMessageBuildUnstable"));

I tried to follow your commit and do the appropriate changes on top of that, but I don't know how to add that HashMap as you did (in your config jelly changes).

Any ideas on that? I'd love to get this #169 commit merged into mainline. It works beautifully.

@timja
Copy link
Member

timja commented Dec 4, 2018

The jelly changes require matching getters and setters, with the setters annotated with @DataboundSetter

@KreAch3R
Copy link
Contributor Author

KreAch3R commented Dec 4, 2018

Ok, so should I try to create a setter that creates that customMessageMap? I'm not sure how jelly will/can handle it, since it's not a simple "value get". It's a Map with values and Results and there is a custom getCustomMessage(String buildType) command that returns the correspondent value.

Any pointers would be appreciated.

@timja
Copy link
Member

timja commented Dec 5, 2018

I've never tried a map, if you want to continue down that path I would ask:
on gitter,
or IRC
or mailing list

otherwise you could store all the fields on the object directly as from looking at the ui it just seemed to be more fields?
and then add a method to retrieve the right message?

KreAch3R added a commit to KreAch3R/slack-plugin that referenced this pull request Dec 16, 2018
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
KreAch3R added a commit to KreAch3R/slack-plugin that referenced this pull request Dec 17, 2018
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
KreAch3R added a commit to KreAch3R/slack-plugin that referenced this pull request Dec 17, 2018
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
timja pushed a commit that referenced this pull request Dec 17, 2018
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants