-
Notifications
You must be signed in to change notification settings - Fork 414
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 baseUrl parameter to support slack-compatible integrations #293
Conversation
Reamer
commented
Mar 2, 2017
- Remove Merge Conflicts from add baseUrl parameter to support slack-compatible integrations #204
- Add some documentation
@Reamer can you amend your PR so that there's two commits? One commit for the author of #204 and a second commit for your additional changes? I prefer to preserve credit where due in the contributor statistics. Actually, since you resolved conflicts that may no longer be easily done. Tell me your thoughts. |
I'll test this change now. |
So far in my testing; upgrading the plugin from 2.1 to 2.2-SNAPSHOT doesn't break any configurations. That's a good start. Now I'm going to review the code. |
I have mattermost configured on Bootstrap Jenkins with slack configured and a mattermost machine (requires vagrant).
Beyond that; I don't know how to test the integration. |
@samrocketman You have now write access to my branch.
Now I will take a look into your Vagrant-Box |
@samrocketman: Vagrant is up and running, but I doesn't want a local jenkins server on my os. I was wondering that the integration button was missing. After a clean install you must enable this integration Let me know ,if I can help you with testing. |
Thanks, I'll try it tomorrow. |
What worksUpgrading plugin preserves settings for existing slack configurations. All plugin settings were preserved when I updated the Base URL. Notifying mattermost was also successful. Here's a screenshot of some output. What doesn't workTest ConnectionSo far adding a trailing slash is required. I configured Base URL = |
trailing slash.
Fully TestedOkay, this has FINALLY been successfully tested. I tested the following:
This pull request is ready to be merged! |
Thanks for your work @Reamer and @andrzejwp. |
Feel free to subscribe to #296 for updates related to Slack Plugin 2.2 release. |
Great! Sorry I didn't asist further. Hope this makes it to the public release soon. Thanks! |
No worries. It's challenging to stay on top of volunteer stuff so I appreciate your contributions. |