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
[bugfix] Fix warnings flagged by closure compiler #4350
[bugfix] Fix warnings flagged by closure compiler #4350
Conversation
621fffd
to
ff411bc
Compare
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.
LGTM
I expect this was a bug and we actually intended the RegExp to search for a literal period. In that case, the correct fix should be to double the backslashes rather than removing them. |
This statement as it stands: this.weekdays(mom, '').replace('.', '\.?') is not correct, and recent closure compiler issues unnecessary escape warning, claiming that '\.' is equivalent to just '.'. It was actually intended the RegExp to search for a literal period. In that case the correct fix would be to double the backslashes rather than removing them.
ff411bc
to
8aa0be6
Compare
Done. |
Can we add a couple tests around this? If the tests pass both ways, then it's not clear what the intended behavior is. |
The latest rules_closure changes updated closure compiler version and remove stale bazel version check, that broke Bazel CI for gerrit: [1]. It turns out that the current code cannot be compiled by the latest closure compiler, so that it must be adjusted first. We add two more suppressions to the closure compiler invocation: * JSC_UNNECESSARY_ESCAPE * JSC_JSDOC_MISSING_TYPE_WARNING Some of the warnings flagged by the closure compiler are related to the moment library. The PR to fix them is pending for review: [2]. * [1] bazelbuild/bazel#4425 * [2] moment/moment#4350 Change-Id: I0354ceddc7f615d93b5d23cf2652b8552e53cc91
The latest rules_closure changes updated closure compiler version and remove stale bazel version check, that broke Bazel CI for gerrit: [1]. It turns out that the current code cannot be compiled by the latest closure compiler, so that it must be adjusted first. We add two more suppressions to the closure compiler invocation: * JSC_UNNECESSARY_ESCAPE * JSC_JSDOC_MISSING_TYPE_WARNING Some of the warnings flagged by the closure compiler are related to the moment library. The PR to fix them is pending for review: [2]. * [1] bazelbuild/bazel#4425 * [2] moment/moment#4350 Change-Id: I0354ceddc7f615d93b5d23cf2652b8552e53cc91
closing in favor of #4453 |
Closes #4347.