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

fix: prevent console logging duplicate deprecation warnings #7733

Merged
merged 9 commits into from
Jan 9, 2024

Conversation

ebrahim95
Copy link
Contributor

@ebrahim95 ebrahim95 commented Dec 20, 2023

The basics

The details

Resolves

Fixes #7719

Proposed Changes

I added the Set Data structure to the deprecation.ts file to prevent logging multiple deprecation warnings.

Reason for Changes

The changes need to be made because if there are multiple warnings that have the same message,
then the warnings will be logged more than once in the console.

Test Coverage

I tested this by adding:
Blockly.utils.deprecation.warn('test warning', 'vA', 'vB', 'test substitute'); Blockly.utils.deprecation.warn('test warning', 'vA', 'vB', 'test substitute');
to the playground.html file.
If the warning appears once then the test was successful.

Documentation

Additional Information

@ebrahim95 ebrahim95 requested a review from a team as a code owner December 20, 2023 00:05
Copy link

conventional-commit-lint-gcf bot commented Dec 20, 2023

🤖 I detect that the PR title and the commit message differ and there's only one commit. To use the PR title for the commit history, you can use Github's automerge feature with squashing, or use automerge label. Good luck human!

-- conventional-commit-lint bot
https://conventionalcommits.org/

@ebrahim95 ebrahim95 changed the title (feat): added Set to prevent console logging multiple deprecation warning #7719 feat!: added Set to prevent console logging multiple deprecation warning #7719 Dec 20, 2023
@github-actions github-actions bot added breaking change Used to mark a PR or issue that changes our public APIs. PR: feature Adds a feature labels Dec 20, 2023
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Welcome! It looks like this is your first pull request in Blockly, so here are a couple of tips:

  • You can find tips about contributing to Blockly and how to validate your changes on our developer site.
  • All contributors must sign the Google Contributor License Agreement (CLA). If the google-cla bot leaves a comment on this PR, make sure you follow the instructions.
  • We use conventional commits to make versioning the package easier. Make sure your commit message is in the proper format or learn how to fix it.
  • If any of the other checks on this PR fail, you can click on them to learn why. It might be that your change caused a test failure, or that you need to double-check the style guide.
    Thank you for opening this PR! A member of the Blockly team will review it soon.

@github-actions github-actions bot added breaking change Used to mark a PR or issue that changes our public APIs. PR: feature Adds a feature and removed PR: feature Adds a feature breaking change Used to mark a PR or issue that changes our public APIs. labels Dec 21, 2023
@cpcallen cpcallen changed the title feat!: added Set to prevent console logging multiple deprecation warning #7719 fix: prevent console logging duplicate deprecation warnings Dec 21, 2023
@github-actions github-actions bot added PR: fix Fixes a bug and removed PR: feature Adds a feature breaking change Used to mark a PR or issue that changes our public APIs. labels Dec 21, 2023
Copy link
Contributor

@cpcallen cpcallen left a comment

Choose a reason for hiding this comment

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

Hi ebrahim95, and thanks for this contribution!

Overall it looks great but there is one small thing that needs to be fixed to comply with our style guide, and one other suggestion I have to make the code easier to read.

(I've also taken the liberty of tweaking some of your comments and the PR title to better fit with our usual style.)

core/utils/deprecation.ts Outdated Show resolved Hide resolved
core/utils/deprecation.ts Outdated Show resolved Hide resolved
ebrahim95 and others added 2 commits December 21, 2023 13:46
Edited By Cpcallen

Co-authored-by: Christopher Allen <cpcallen+github@gmail.com>
Edited By Cpcallen

Co-authored-by: Christopher Allen <cpcallen+github@gmail.com>
@ebrahim95
Copy link
Contributor Author

@cpcallen I appreciate the suggestions 😄
They have helped me understand how to write readable code.

Copy link
Contributor

@cpcallen cpcallen left a comment

Choose a reason for hiding this comment

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

Looks like you didn't update the other places that checkMsg is used, and now there are syntax errors.

@cpcallen cpcallen merged commit 2ebc6e1 into google:develop Jan 9, 2024
6 checks passed
@ebrahim95 ebrahim95 deleted the dev_changes branch January 17, 2024 06:26
@ebrahim95 ebrahim95 restored the dev_changes branch January 17, 2024 06:27
@ebrahim95 ebrahim95 deleted the dev_changes branch January 17, 2024 06:27
@ebrahim95 ebrahim95 restored the dev_changes branch January 17, 2024 06:27
@github-actions github-actions bot added PR: fix Fixes a bug and removed PR: fix Fixes a bug labels Jan 19, 2024
@ebrahim95 ebrahim95 deleted the dev_changes branch January 23, 2024 03:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: fix Fixes a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Prevent deprecation.warn from filing duplicate warnings
2 participants