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
refactor(common): make the error messages tree shakable #44663
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ramthir thanks for creating this PR 👍 Just left a couple comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ramthir thanks for additional updates, please see a couple comments below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ramthir the changes look good, thanks 👍
It looks like there are few places in tests that require some updates (see CI job output here) due to an error code being present in the error message. Could you please take a look when you get a chance?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ramthir thanks for additional updates, the changes look good 👍
Note: this PR would require a rebase once PR #44677 lands. PR #44677 adds golden files for error message codes (for extra protection from accidental changes) and one of the golden files would require an update to include a new error code. I will let you know once PR #44677 lands.
Thank you.
@ramthir thanks for rebasing this PR. Could you please also run the following command (as suggested by the CI output):
and include the changes into the commit? That should make the CI "green". Thank you. |
Make Long error messages tree-shakable in the production build with error codes. fixes #40096
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ramthir thanks for applying additional changes! 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed-for: public-api
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reviewed-for: public-api
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reviewed-for: public-api
This PR was merged into the repository by commit 4307b82. |
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Make Long error messages tree-shakable in the production build with error codes.
fixes #40096
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Invalid argument errors in the pipes are not tree-shakable in the production build.
Issue Number: 40096
What is the new behavior?
Make the invalid argument errors in the pipes tree-shakable in the production build.
Does this PR introduce a breaking change?
Other information