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

deprecation(log): rename warning() to warn() #4117

Merged
merged 19 commits into from
Jan 11, 2024

Conversation

timreichen
Copy link
Contributor

ref: #4099

@timreichen timreichen requested a review from kt3k as a code owner January 5, 2024 10:56
@github-actions github-actions bot added the log label Jan 5, 2024
log/test.ts Show resolved Hide resolved
Copy link
Collaborator

@iuioiua iuioiua left a comment

Choose a reason for hiding this comment

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

Nice to see this done! Few nits.

log/logger.ts Show resolved Hide resolved
log/logger.ts Show resolved Hide resolved
log/mod.ts Show resolved Hide resolved
log/mod.ts Outdated Show resolved Hide resolved
log/test.ts Show resolved Hide resolved
@github-actions github-actions bot added the semver label Jan 7, 2024
@github-actions github-actions bot removed the semver label Jan 7, 2024
Copy link
Collaborator

@iuioiua iuioiua left a comment

Choose a reason for hiding this comment

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

LGTM! Nice.

Am I correct in understanding that this change is non-breaking?

@kt3k
Copy link
Member

kt3k commented Jan 8, 2024

Am I correct in understanding that this change is non-breaking?

Strictly speaking, if the users do any log analysis using WARNING symbol, then that analysis will break. Can we delay the change of output string from WARNING to WARN to the actual breaking version? (after 0.214.0)

@github-actions github-actions bot removed the testing label Jan 8, 2024
@timreichen
Copy link
Contributor Author

Am I correct in understanding that this change is non-breaking?

Strictly speaking, if the users do any log analysis using WARNING symbol, then that analysis will break. Can we delay the change of output string from WARNING to WARN to the actual breaking version? (after 0.214.0)

Reverted

@kt3k
Copy link
Member

kt3k commented Jan 9, 2024

@timreichen I don't see well what part is reverted?

The below still seems printing WARN hi

import * as log from "./log/mod.ts";
log.warning("hi");

@timreichen
Copy link
Contributor Author

@timreichen I don't see well what part is reverted?

The below still seems printing WARN hi

import * as log from "./log/mod.ts";
log.warning("hi");

Sorry, seems like the commit wasn't pushed. Done now.

Copy link
Member

@kt3k kt3k left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@iuioiua iuioiua left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you.

@iuioiua iuioiua merged commit f5b6774 into denoland:main Jan 11, 2024
12 checks passed
@timreichen timreichen deleted the log_rename_waring_to_warn branch January 11, 2024 08:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants