The Wayback Machine - https://web.archive.org/web/20221028004342/https://github.com/paritytech/substrate/pull/7281
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

Fixes logging of target names with dashes #7281

Merged
3 commits merged into from Oct 8, 2020

Conversation

bkchr
Copy link
Member

@bkchr bkchr commented Oct 8, 2020

There was a bug in tracing-core which resulted in not supporting dashes
in target names. This was fixed upstream. Besides that a test was added
to ensure that we don't break this again.

Fixes: #7251

There was a bug in tracing-core which resulted in not supporting dashes
in target names. This was fixed upstream. Besides that a test was added
to ensure that we don't break this again.
@bkchr bkchr added A0-pleasereview Pull request needs code review. B5-clientnoteworthy Changes should be mentioned in any downstream projects' release notes C3-medium 📣 Elevates a release containing this PR to "medium priority". labels Oct 8, 2020
@bkchr bkchr requested review from andresilva and tomaka Oct 8, 2020
const EXPECTED_LOG_MESSAGE: &'static str = "yeah logging works as expected";

#[test]
fn dash_in_target_name_works() {
Copy link
Member

@andresilva andresilva Oct 8, 2020

Choose a reason for hiding this comment

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

Nice test :)

@@ -371,4 +372,33 @@ mod tests {
assert!(test_filter("telemetry", Level::TRACE));
Copy link
Member

@andresilva andresilva Oct 8, 2020

Choose a reason for hiding this comment

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

Maybe we can also add an example with dashes to this test 🤷

Copy link
Member Author

@bkchr bkchr Oct 8, 2020

Choose a reason for hiding this comment

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

Added

@bkchr
Copy link
Member Author

bkchr commented Oct 8, 2020

bot merge

@ghost
Copy link

ghost commented Oct 8, 2020

Waiting for commit status.

@github-actions github-actions bot added the A7-needspolkadotpr Pull request is reviewed well, but cannot be merged until there is a corresponding Polkadot PR. label Oct 8, 2020
@ghost
Copy link

ghost commented Oct 8, 2020

Checks failed; merge aborted.

tomaka
tomaka approved these changes Oct 8, 2020
@bkchr
Copy link
Member Author

bkchr commented Oct 8, 2020

bot merge

@ghost
Copy link

ghost commented Oct 8, 2020

Waiting for commit status.

@github-actions github-actions bot removed the A7-needspolkadotpr Pull request is reviewed well, but cannot be merged until there is a corresponding Polkadot PR. label Oct 8, 2020
@ghost ghost merged commit ffe1540 into master Oct 8, 2020
20 checks passed
@ghost ghost deleted the bkchr-fix-dash-target-name-logging branch Oct 8, 2020
ordian added a commit that referenced this pull request Oct 9, 2020
…up-updates

* master:
  Async keystore + Authority-Discovery async/await (#7000)
  Fixes logging of target names with dashes (#7281)
  seal: Add automated weights for contract API calls (#7017)
  add ss58 id for nodle (#7279)
  Refactor CurrencyToVote (#6896)
  bump-allocator: document & poison (#7277)
  Reset flaming fir network (#7274)
  reschedule (#6860)
  Drop system cache for trie benchmarks (#7242)
  client: improve log formatting (#7272)
  Rework `InspectState` (#7271)
  SystemOrigin trait (#7226)
  Update ss58 registry for Dock network (#7263)
  .maintain/monitoring: Add alert when continuous task ends (#7250)
  Rename `TRANSACTION_VERSION` to `EXTRINSIC_VERSION` (#7258)
  Split block announce processing into two parts (#6958)
  Fix offchain election to respect the weight (#7215)
This pull request was closed.
Labels
A0-pleasereview Pull request needs code review. B5-clientnoteworthy Changes should be mentioned in any downstream projects' release notes C3-medium 📣 Elevates a release containing this PR to "medium priority".
3 participants