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

chore: remove ng lint in angular example #2072

Merged
merged 3 commits into from
Nov 13, 2023

Conversation

alicialics
Copy link
Contributor

@alicialics alicialics commented Nov 10, 2023

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

@alicialics alicialics requested a review from a team as a code owner November 10, 2023 19:14
@alicialics alicialics requested review from cpcallen and removed request for a team November 10, 2023 19:14
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.

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.

@cpcallen cpcallen requested a review from maribethb November 13, 2023 18:52
@cpcallen cpcallen removed their assignment Nov 13, 2023
Copy link
Contributor

@maribethb maribethb left a 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.

@alicialics alicialics changed the title chore: fix ng lint in angular example chore: remove ng lint in angular example Nov 13, 2023
Copy link
Contributor

@maribethb maribethb left a 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!

@maribethb maribethb merged commit bd447f5 into google:master Nov 13, 2023
10 checks passed
@alicialics alicialics deleted the fix_angular_lint branch November 14, 2023 01:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants