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.

Sign up for GitHub

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

Jump to bottom

Fix log message id from at-deprecate to be a Symbol. #40820

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix log message id from at-deprecate to be a Symbol. #40820

wants to merge 1 commit into from

Conversation

fredrikekre
Copy link
Member

No description provided.

@fredrikekre fredrikekre added logging The logging framework backport 1.6 Change should be backported to release-1.6 labels May 13, 2021
@fredrikekre fredrikekre requested a review from c42f May 17, 2021 07:29
@fredrikekre
Copy link
Member Author

Tests fail because of top-level ids now beeing the same, xref #25158. Currently those tests pass because of === fallback comparing two identical frames, which seems brittle too. Should the patch in this PR instead generate a id from a hash of the frame or something?

@KristofferC KristofferC mentioned this pull request Jun 4, 2021
Merged
45 tasks
@vtjnash vtjnash closed this Jun 19, 2021
@vtjnash vtjnash reopened this Jun 19, 2021
@vtjnash
Copy link
Sponsor Member

vtjnash commented Jun 19, 2021

Can you explain briefly what issue this will fix? The description above is blank. Re-running CI now, since we recently suffered a loss of all logs.

@fredrikekre
Copy link
Member Author

I had a log record struct with ::Symbol on the message id field but that broke for deprecation log messages. Perhaps the assumption that the id is always a symbol was wrong though?

@KristofferC KristofferC mentioned this pull request Jul 12, 2021
Merged
75 tasks
@KristofferC KristofferC mentioned this pull request Aug 25, 2021
Merged
63 tasks
@KristofferC KristofferC mentioned this pull request Oct 19, 2021
Merged
95 tasks
@KristofferC KristofferC mentioned this pull request Nov 8, 2021
Merged
66 tasks
@KristofferC KristofferC mentioned this pull request Nov 19, 2021
Merged
29 tasks
@fredrikekre fredrikekre removed the backport 1.6 Change should be backported to release-1.6 label Dec 2, 2021
@fredrikekre fredrikekre mentioned this pull request Dec 16, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@c42f c42f Awaiting requested review from c42f

Assignees
No one assigned
Labels
logging The logging framework
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@fredrikekre @vtjnash