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

~The RTI should accept an argument to set the log level~ #8

Closed
lhstrh opened this issue Oct 20, 2021 · 4 comments
Closed

~The RTI should accept an argument to set the log level~ #8

lhstrh opened this issue Oct 20, 2021 · 4 comments
Labels
enhancement Enhancement of existing feature good first issue Good for newcomers

Comments

@lhstrh
Copy link
Member

lhstrh commented Oct 20, 2021

Currently, recompilation is required to change the log level. It would be much better if this could be done using a CLI argument.

@lhstrh lhstrh added enhancement Enhancement of existing feature good first issue Good for newcomers labels Oct 20, 2021
@edwardalee
Copy link
Contributor

I don't think it's a good idea to support this as a CLI. Logging can have considerable impact on performance, and if it's a CLI, then at a minimum, every logging point incurs the penalty of a test. If it is implemented badly, it will also incur the penalty of constructing the logging message that it will then not print.

@lhstrh
Copy link
Member Author

lhstrh commented Oct 20, 2021

That's a good point -- I hadn't thought of that. Rather than telling folks to go edit the source, however, how about we create a build option for this?

@Soroosh129
Copy link
Contributor

You don't need to edit the source code. This is explained in the RTI's README file:

Note: To enable DEBUG messages, use the following build commands instead:

mkdir build && cd build
cmake -DCMAKE_BUILD_TYPE=DEBUG ../
make
sudo make install

@lhstrh
Copy link
Member Author

lhstrh commented Oct 20, 2021

Oh, great! My suggestion was prompted by this comment, but this certainly addresses it.

@lhstrh lhstrh closed this as completed Oct 20, 2021
@lhstrh lhstrh changed the title The RTI should accept an argument to set the log level ~~The RTI should accept an argument to set the log level~~ Oct 20, 2021
@lhstrh lhstrh changed the title ~~The RTI should accept an argument to set the log level~~ ~The RTI should accept an argument to set the log level~ Oct 20, 2021
@lhstrh lhstrh changed the title ~The RTI should accept an argument to set the log level~ The RTI should accept an argument to set the log level Oct 20, 2021
@lhstrh lhstrh changed the title The RTI should accept an argument to set the log level ~The RTI should accept an argument to set the log level~ Oct 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement of existing feature good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants