-
Notifications
You must be signed in to change notification settings - Fork 25.1k
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
build: complete removal of bazel format yarn commands #37148
build: complete removal of bazel format yarn commands #37148
Conversation
c8998a1
to
50f17e9
Compare
@@ -16,9 +16,6 @@ | |||
"url": "https://github.com/angular/angular.git" | |||
}, | |||
"scripts": { | |||
"bazel:format": "yarn -s ng-dev format deprecation-warning bazel:format", | |||
"bazel:lint": "yarn -s ng-dev format deprecation-warning bazel:lint", | |||
"bazel:lint-fix": "yarn -s ng-dev format deprecation-warning bazel:lint-fix", |
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.
How about we keep these in here for a bit longer, but instead of displaying a warning, we error?
TBH I didn't catch that we should be now using ng-dev format
instead of yarn bazel:format
and if this got merged without me randomly opening this PR I wouldn't know what to do next time I need to reformat some files.
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.
Currently, we already fail to perform the format and instead instruct the user to run the new command.
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.
you are right, I must have been lucky and not have a need to run the script because my IDE formatter does the job and I rarely need to reformat from the command line, but when I do I'd prefer to at least see a comment in package.json under scripts that reminds me to use yarn ng-dev --help
instead.
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.
Added general note in "scripts"
about our increased migration to using ng-dev
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.
thank you!
Remove bazel yarn format deprecation message to complete the removal of formatting bazel related files via yarn command command.
50f17e9
to
5b004e4
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!
Remove bazel yarn format deprecation message to complete the removal of formatting bazel related files via yarn command command. PR Close #37148
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. |
Remove bazel yarn format deprecation message to complete the removal of formatting bazel related files via yarn command command. PR Close angular#37148
Remove bazel yarn format deprecation message to complete the removal of
formatting bazel related files via yarn command command.