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.

Sign up for GitHub

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

Jump to bottom

feat(prefer-implicit-assert): adding new rule #815

Merged
merged 9 commits into from
Oct 12, 2023
Merged

feat(prefer-implicit-assert): adding new rule #815

merged 9 commits into from
Oct 12, 2023

Conversation

adevick
Copy link
Contributor

@adevick adevick commented Sep 11, 2023 •
edited by Belco90

Checks

Changes

  • Adding a new rule to prefer implicit assertions when using findBy* and getBy* queries

Context

Closes #743

@adevick adevick mentioned this pull request Sep 11, 2023
Closed
@adevick adevick changed the title feat(prefer-explicit-assert): adding new rule feat(prefer-implicit-assert): adding new rule Sep 11, 2023
@Belco90 Belco90 self-requested a review September 12, 2023 06:56
@Belco90 Belco90 added the new rule New rule to be included in the plugin label Sep 12, 2023
@adevick
Copy link
Contributor Author

adevick commented Sep 20, 2023

@Belco90 just checking in thanks!

@Belco90
Copy link
Member

Belco90 commented Sep 21, 2023

I'll try to review this next week.

Belco90
Belco90 requested changes Sep 29, 2023
View reviewed changes
Copy link
Member

@Belco90 Belco90 left a comment

Choose a reason for hiding this comment

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

Looks good in general! I requested a couple of changes that should be simple to apply, and should be ready to go.

docs/rules/prefer-implicit-assert.md Show resolved Hide resolved
docs/rules/prefer-explicit-assert.md Show resolved Hide resolved
tests/lib/rules/prefer-implicit-assert.test.ts Show resolved Hide resolved
lib/rules/prefer-implicit-assert.ts Outdated Show resolved Hide resolved
lib/rules/prefer-implicit-assert.ts Outdated Show resolved Hide resolved
lib/rules/prefer-implicit-assert.ts Show resolved Hide resolved
@adevick adevick requested a review from Belco90 October 6, 2023 21:23
@adevick
Copy link
Contributor Author

adevick commented Oct 6, 2023

@Belco90 Hey thanks for the review, fixed all but the last comment.

Belco90
Belco90 previously approved these changes Oct 10, 2023
View reviewed changes
Copy link
Member

@Belco90 Belco90 left a comment •
edited

Choose a reason for hiding this comment

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

LGTM, but it seems you need some extra tests to reach 90% of coverage.

@adevick
Copy link
Contributor Author

adevick commented Oct 12, 2023

@Belco90 addresssed test coverage, turns out I didn't need those extra blocks of code after the refactor, and should be all green now. Thanks!

@Belco90 Belco90 merged commit 56a1900 into testing-library:main Oct 12, 2023
29 checks passed
@github-actions
Copy link

🎉 This PR is included in version 6.1.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@Belco90
Copy link
Member

Belco90 commented Oct 12, 2023

@all-contributors please add @adevick for code, test, docs

@allcontributors allcontributors bot mentioned this pull request Oct 12, 2023
Merged
@allcontributors
Copy link
Contributor

@Belco90

I've put up a pull request to add @adevick! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Belco90 Belco90 Belco90 approved these changes

Assignees

@adevick adevick

Labels
new rule New rule to be included in the plugin released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Unnecessary presence checks on findBy queries
2 participants
@adevick @Belco90