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

feat: Add version log #86

Merged
merged 1 commit into from
Oct 5, 2023
Merged

feat: Add version log #86

merged 1 commit into from
Oct 5, 2023

Conversation

fmoura
Copy link
Contributor

@fmoura fmoura commented Sep 14, 2023

Adding as a Draft to check if the way the "Versioning" Log appears now is the expected one

@tuler
Copy link
Member

tuler commented Sep 14, 2023

Avoid recording time of build. It goes against reproducible build motto.

@fmoura fmoura force-pushed the feature/log-version branch from 152cb11 to dcfcf88 Compare September 14, 2023 16:46
offchain/Cargo.toml Outdated Show resolved Hide resolved
offchain/authority-claimer/Cargo.toml Outdated Show resolved Hide resolved
offchain/authority-claimer/Cargo.toml Outdated Show resolved Hide resolved
@fmoura fmoura force-pushed the feature/log-version branch 8 times, most recently from af6473e to bcca7cd Compare September 21, 2023 20:08
@fmoura fmoura force-pushed the feature/log-version branch from bcca7cd to 39cdec0 Compare September 22, 2023 20:04
@fmoura fmoura self-assigned this Sep 22, 2023
@fmoura fmoura force-pushed the feature/log-version branch 2 times, most recently from fc41bad to 1cb321f Compare September 23, 2023 00:03
@fmoura fmoura marked this pull request as ready for review September 23, 2023 00:20
Copy link
Contributor

@gligneul gligneul left a comment

Choose a reason for hiding this comment

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

Looking good! Just minor comments.

CHANGELOG.md Outdated Show resolved Hide resolved
offchain/Cargo.toml Outdated Show resolved Hide resolved
build/Dockerfile Outdated Show resolved Hide resolved
offchain/graphql-server/Cargo.toml Outdated Show resolved Hide resolved
offchain/host-runner/Cargo.toml Outdated Show resolved Hide resolved
offchain/dispatcher/src/built/build.rs Outdated Show resolved Hide resolved
offchain/log/Cargo.toml Outdated Show resolved Hide resolved
offchain/log/src/lib.rs Outdated Show resolved Hide resolved
offchain/log/src/lib.rs Outdated Show resolved Hide resolved
offchain/log/src/lib.rs Outdated Show resolved Hide resolved
offchain/Cargo.toml Outdated Show resolved Hide resolved
offchain/graphql-server/Cargo.toml Outdated Show resolved Hide resolved
offchain/inspect-server/Cargo.toml Outdated Show resolved Hide resolved
offchain/indexer/src/main.rs Outdated Show resolved Hide resolved
offchain/host-runner/Cargo.toml Outdated Show resolved Hide resolved
offchain/log/src/lib.rs Outdated Show resolved Hide resolved
@fmoura fmoura force-pushed the feature/log-version branch 3 times, most recently from 460982a to dabd72f Compare September 29, 2023 02:24
@fmoura fmoura changed the base branch from main to next September 29, 2023 14:04
@@ -25,7 +25,7 @@ pub async fn start(
config: DispatcherConfig,
metrics: DispatcherMetrics,
) -> Result<(), DispatcherError> {
info!("Setting up dispatcher with config: {:?}", config);
info!("Setting up dispatcher");
Copy link
Contributor

Choose a reason for hiding this comment

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

We could remove this log since it doesn't add much else.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure. Could that be a "trace" instead of "info" then, as logs also acts as "execution markers" that helps spot bugs. What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

Changing it to trace is fine as well.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@gligneul @fmoura, are we going to keep the info instead of changing it to a trace?

@fmoura fmoura force-pushed the feature/log-version branch 2 times, most recently from 0a699be to d30ef2a Compare September 29, 2023 19:15
@fmoura
Copy link
Contributor Author

fmoura commented Oct 1, 2023

close #8

@fmoura fmoura linked an issue Oct 1, 2023 that may be closed by this pull request
7 tasks
CHANGELOG.md Outdated Show resolved Hide resolved
@fmoura fmoura force-pushed the feature/log-version branch from d30ef2a to cd21dcd Compare October 3, 2023 19:41
marcelstanley
marcelstanley previously approved these changes Oct 3, 2023
@gligneul gligneul changed the base branch from next to main October 4, 2023 13:28
@gligneul gligneul dismissed marcelstanley’s stale review October 4, 2023 13:28

The base branch was changed.

@fmoura fmoura force-pushed the feature/log-version branch 4 times, most recently from 9ec5208 to c9f94e4 Compare October 4, 2023 20:42
CHANGELOG.md Show resolved Hide resolved
@fmoura fmoura force-pushed the feature/log-version branch from cffbcb3 to 706527c Compare October 5, 2023 13:50
@fmoura fmoura requested a review from gligneul October 5, 2023 14:11
@fmoura fmoura merged commit 61b58a5 into main Oct 5, 2023
6 checks passed
@fmoura fmoura deleted the feature/log-version branch October 5, 2023 21:25
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.

Log version in node startup
4 participants