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

Don't allow both original and JSON commands to be registered simultaneously #15

Merged
merged 2 commits into from
May 30, 2024

Conversation

anjohnson
Copy link
Member

Fixes #14 and #10, warns user of bad IOC configurations.

R4-1: Changes since R4-0
------------------------

**This version has not yet been released**
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NB: Remove this comment before releasing.
Add other PR changes to these notes too...

@simon-ess
Copy link
Contributor

@anjohnson - while I think it's a good idea to prevent both json/non-json logs being initialised at the same time, I have to admit that I find it a bit odd that we are making this restriction at build-time instead of only at run-time. Wouldn't it be more flexible implement only the check that prevents the registration of both json/non-json logging tasks? Otherwise if you want to change your logging you need to recompile the entire application or library, instead of simply changing the configuration in the startup script.

If someone wants that flexibility, it seems a bit restrictive to disallow it...

Log/print warning or error messages to explain what's wrong.

Fixes GitHub issue epics-modules#14
@anjohnson
Copy link
Member Author

@simon-ess Thanks for the comment, but please read the discussions in issues #8, #10 and #14 which document how we got to this point. If a site does want to be able to choose which logging format to use at init-time they can submit a PR for that, but it's not trivial to do.

The original logging format is not properly parsable, which is why I want it to die out.

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.

Using caPutLogShow after caPutJsonLogInit causes segfault
2 participants