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

refactor(eslint-plugin-template): no-negated-async no longer perform equality checks #399

Conversation

rafaelss95
Copy link
Member

BREAKING CHANGE.

Currently it's a proposal. If it gets approved, we could implement a rule to perform equality checks.

Fixes #315.

@rafaelss95 rafaelss95 added the BREAKING CHANGE This is a breaking change and should only be released as part of a new major version label Apr 7, 2021
@rafaelss95 rafaelss95 marked this pull request as draft April 7, 2021 16:52
@nx-cloud
Copy link

nx-cloud bot commented Apr 7, 2021

Nx Cloud Report

CI ran the following commands for commit 1766ce5. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

Status Command
#000000 nx run-many --target=build --all --parallel
#000000 nx run-many --target=check-configs --all --parallel
#000000 nx run-many --target=test --all --parallel
#000000 nx run-many --target=typecheck --all --parallel

Sent with 💌 from NxCloud.

@nx-cloud
Copy link

nx-cloud bot commented Apr 7, 2021

Nx Cloud Report

CI ran the following commands for commit 42dccdd. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

Status Command
#000000 nx run-many --target=build --all --parallel
#000000 nx run-many --target=check-configs --all --parallel
#000000 nx run-many --target=test --all --parallel
#000000 nx run-many --target=typecheck --all --parallel

Sent with 💌 from NxCloud.

Copy link
Member

@JamesHenry JamesHenry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah seems like a good idea, and we'll definitely want to ensure we don't lose functionality overall. So:

@JamesHenry
Copy link
Member

I'd like to get v3 out as soon as we can so going to mark this one for v12 (the next major version to align us with the next major angular version)

@JamesHenry JamesHenry added this to the v12 milestone Apr 11, 2021
@JamesHenry
Copy link
Member

JamesHenry commented May 12, 2021

@rafaelss95 No worries if not, but do you think you'll be able to work on this this week? Angular 12 is getting released today and I'd like to follow it up with @angular-eslint 12 stable as soon as we can.

This will be our last breaking change until Angular 13 so that we can stay aligned with Angular's own major version to make it really straightforward for users

@rafaelss95
Copy link
Member Author

rafaelss95 commented May 13, 2021

@rafaelss95 No worries if not, but do you think you'll be able to work on this this week? Angular 12 is getting released today and I'd like to follow it up with @angular-eslint 12 stable as soon as we can.

This will be our last breaking change until Angular 13 so that we can stay aligned with Angular's own major version to make it really straightforward for users

Oh, I'll try to do my best. PR for the new rule is open: #444. I'll try to work on a migration now.

Btw, what should the migration do? Add the new equality rule?

@JamesHenry
Copy link
Member

@rafaelss95 Yes exactly, whatever it takes to achieve no change in behaviour when running ng lint on v4 vs v12

@JamesHenry JamesHenry changed the base branch from master to next-major-release/v12 May 13, 2021 11:39
@JamesHenry
Copy link
Member

I have changed the base branch to be the v12 one, which already has the start of the migration in for v12 so you just need to add to that

@JamesHenry
Copy link
Member

I've turned my previous comment into a TODO list

@JamesHenry
Copy link
Member

@rafaelss95 Are you clear on the next steps above?

@JamesHenry
Copy link
Member

Thanks @rafaelss95 I've updated the list with the progess, I will open the PR to tslint-to-eslint-config

@JamesHenry
Copy link
Member

We can apply the updated tslint-to-eslint-config outside this branch, so please go ahead and get this PR ready for final review

@rafaelss95 rafaelss95 force-pushed the refactor/no-negated-async-pipe-remove-strict-equality branch from 42dccdd to 1766ce5 Compare May 13, 2021 16:17
@rafaelss95 rafaelss95 changed the title [WIP] refactor(eslint-plugin-template): no-negated-async no longer perform equality checks refactor(eslint-plugin-template): no-negated-async no longer perform equality checks May 13, 2021
@rafaelss95 rafaelss95 marked this pull request as ready for review May 13, 2021 16:18
@JamesHenry JamesHenry merged commit 767dc93 into angular-eslint:next-major-release/v12 May 13, 2021
@rafaelss95 rafaelss95 deleted the refactor/no-negated-async-pipe-remove-strict-equality branch May 13, 2021 16:32
JamesHenry pushed a commit that referenced this pull request May 13, 2021
…ality checks (#399)

BREAKING CHANGE

You should add the new `@angular-eslint/template/eqeqeq` rule to your config if you want to continue with the same functionality around equality checks. This will be applied for you by `ng update` automatically.
JamesHenry pushed a commit that referenced this pull request May 13, 2021
…ality checks (#399)

BREAKING CHANGE

You should add the new `@angular-eslint/template/eqeqeq` rule to your config if you want to continue with the same functionality around equality checks. This will be applied for you by `ng update` automatically.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BREAKING CHANGE This is a breaking change and should only be released as part of a new major version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[no-negated-async]: false positives with equality report
2 participants