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

HTML mail notifications fix and improvements #1746

Merged
merged 13 commits into from
Jan 15, 2016

Conversation

mohierf
Copy link
Contributor

@mohierf mohierf commented Nov 3, 2015

Allow to set the mail sender as to not always use root@shinken.main
- search the logo in WebUI2 configuration
- search WebUI port in WebUI2 configuration
- added an option to set WebUI URL
@olivierHa
Copy link
Collaborator

I've merged another PR with the sender option. I will merge this PR soon, after new travis tests

@mohierf
Copy link
Contributor Author

mohierf commented Dec 11, 2015

I am quite disappointed with your merge strategy ... amongst the 3 PR dealing with a sender option, you choose not to merge this one which also included some fixes, a better integration with Web UI and some doc ! What a strange choice ...

@olivierHa
Copy link
Collaborator

Yes i am sorry about this one. My strategy was simple : start with the
small PR. I haven't expected to see 4 PR dealing with the sender
parameters.
Le 11 déc. 2015 5:20 AM, "Frédéric MOHIER" notifications@github.com a
écrit :

I am quite disappointed with your merge strategy ... amongst the 3 PR
dealing with a sender option, you choose not to merge this one which also
included some fixes, a better integration with Web UI and some doc ! What a
strange choice ...


Reply to this email directly or view it on GitHub
#1746 (comment).

@hvad
Copy link
Collaborator

hvad commented Dec 24, 2015

Why include that on shinken core ? I think you must use mod -> https://shinken.readthedocs.org/en/latest/14_contributing/modules.html.

@olivierHa
Copy link
Collaborator

@mohierf , could you manage the conflicts (again sorry about that :( ).
so i can merge this PR :)

@mohierf
Copy link
Contributor Author

mohierf commented Jan 11, 2016

@olivierHa : I do not see any conflicts anymore now ...

@mohierf
Copy link
Contributor Author

mohierf commented Jan 13, 2016

@olivierHa : I modified the PR and I see that there are still conflicts ... but I do not find them !

…-notification

# Conflicts:
#	libexec/notify_by_email.py
- use "Service state duration" instead of "Service duration"
- use "Host state duration" instead of "Host duration"
@mohierf
Copy link
Contributor Author

mohierf commented Jan 15, 2016

I included #1749 modifications in the last commit of this PR.

olivierHa added a commit that referenced this pull request Jan 15, 2016
HTML mail notifications fix and improvements
@olivierHa olivierHa merged commit 0a6a420 into shinken-solutions:master Jan 15, 2016
@olivierHa
Copy link
Collaborator

Thanks a lot !

mohierf added a commit to mohierf/shinken that referenced this pull request Jan 16, 2016
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.

Mail notification - encoding exception not catched
3 participants