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

Move syslog to included_applications #6

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

aaronjensen
Copy link
Contributor

@aaronjensen aaronjensen commented May 18, 2016

This simplifies setup and seems to work. I think it's more correct.

This supersedes #5

@aaronjensen aaronjensen changed the title Do not start syslog ourselves [DO NOT MERGE YET] Do not start syslog ourselves May 18, 2016
It will be started by us. We also should not include logger in our
applications because we don't want to start it before we start.
@aaronjensen aaronjensen force-pushed the do-not-start-syslog branch from 9fecd50 to 9049023 Compare May 18, 2016 02:17
@aaronjensen aaronjensen changed the title [DO NOT MERGE YET] Do not start syslog ourselves Move syslog to included_applications May 18, 2016
@aaronjensen
Copy link
Contributor Author

@bitwalker does this seem right to you? We can include exsyslog as a normal library application (I'm not sure if it's a problem that ExSyslog itself is a GenEvent, is it?) and then syslog is in included_applications and we start it ourselves.

I wasn't able to get it working conssitently when I put syslog in applications and relied on it starting before logger would try to init ExSyslog.

@aaronjensen
Copy link
Contributor Author

I tested this locally and deployed via exrm, both worked well.

@bitwalker
Copy link

Yep this looks exactly right :)

@aaronjensen
Copy link
Contributor Author

Awesome, thanks @bitwalker. @mr-bt look good?

@RobinClowers
Copy link

@mr-bt any reason we can't get this merged?

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.

3 participants