-
Notifications
You must be signed in to change notification settings - Fork 63
log rotation #48
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
log rotation #48
Conversation
ltitanb
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just a couple of small changes needed, then gtg. Also make sure to test this locally to make sure everything works as expected
crates/common/src/module_names.rs
Outdated
| pub const BUILDER_LOG_NAME: &str = "builder_log"; | ||
| pub const CUSTOM_BOOST_NAME: &str = "custom_boost"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's remove these as they are just examples. For external modules, we should use the module id from the module config. Let's move PBS_MODULE_NAME and SIGNER_MODULE_NAME to config/log.rs
examples/builder_log/Cargo.toml
Outdated
|
|
||
| [dependencies] | ||
| commit-boost = { path = "../../bin" } | ||
| cb-common = {path="../../crates/common"} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
module should only ever import commit-boost and no "internal" crates, so we re-export everything needed in commit_boost::prelude::*. In this case seems that we only used it for the BUILDER_LOG_NAME. Given that this is just an example of a module, we should have them hardcoded in the repo, let's use the module id from the config, which all modules have
tbd: log rotation policy, simplify #47