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

Update: Mark no-assert-equal and no-assert-ok as deprecated #201

Closed
wants to merge 1 commit into from

Conversation

platinumazure
Copy link
Owner

Replaced by no-loose-assertions, which covers these rules but is also more flexible.

@coveralls
Copy link

coveralls commented Aug 25, 2021

Coverage Status

Coverage remained the same at 100.0% when pulling c95fe32 on deprecate-old-loose-assertions into fce1e12 on master.

@platinumazure
Copy link
Owner Author

@bmish Would you mind reviewing this? I added some logic around marking rules as deprecated (and replaced by others), curious if you think I could improve it.

@@ -18,6 +18,7 @@ const assert = require("chai").assert,
//------------------------------------------------------------------------------

const MESSAGES = {
deprecated: "**This rule has been deprecated.** Please see the project README for information about replacement rules.",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's follow the format of the other messages by starting this with an emoji: ❌

@@ -18,6 +18,7 @@ const assert = require("chai").assert,
//------------------------------------------------------------------------------

const MESSAGES = {
deprecated: "**This rule has been deprecated.** Please see the project README for information about replacement rules.",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's add a link to the deprecated section of the README in the README word.

@bmish
Copy link
Collaborator

bmish commented Aug 25, 2021

I think we need to improve no-loose-assertions before it will be ready to replace the other two rules:

I can work on this fixing these issues in the coming weeks.

We would then want to make no-loose-assertions recommended as part of the V7 release (#175).

CC: @raycohen about this move to consolidate on no-loose-assertions.

@platinumazure
Copy link
Owner Author

I had forgotten about marking no-assert-equal as recommended. That does make things a bit more complicated. Managed to miss that in the Project as well.

I'll make a post in the 7.0.0 issue about how we want to handle this.

@platinumazure
Copy link
Owner Author

I'm removing this from 7.0.0 and considering this on hold for now. We can't deprecate until no-loose-assertions properly handles globals.

I would love to get the "infrastructure" for deprecated rules in at some point, but that doesn't have to happen in 7.0.0 nor in this PR.

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