-
Notifications
You must be signed in to change notification settings - Fork 628
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
chore: remove ng lint in angular example #2072
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.
I'm struck by how many dev dependencies this adds; I don't know enough about linting agular projects but it is a surprisingly large number.
Mainly, though, I think I should defer to @maribethb on whether this is the right approach, so I'm going to reassign it to her.
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.
Is it possible to just remove the linting entirely, instead of updating it?
This is a great example of why I'm leaning towards not keeping the angular example at all. Unfortunately, nobody on our team has the expertise to give this a fair review or to keep it up to date. I don't want to pretend to folks who see this example that we know what we're doing and this is a good linting setup they should follow (it might be in this PR, but will it be 2 years from now?). Developers who want to understand how to lint angular projects should seek advice from angular docs, not our demos. Removing linting entirely if that's an option would focus the demo more clearly on the blockly part of it instead of being a full-fledged angular demo.
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! This should be a lot easier to maintain for us!
The basics
ng lint was broken because tslint is deprecated, this PR removes tslint all together
The details
Resolves
#1679
Proposed Changes
ran automatic migrations
Reason for Changes
fixes lint command
Test Coverage
ran lint
Documentation
N/A
Additional Information
N/A