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

feat(rule): add data-testid rule #56

Merged
merged 5 commits into from
Jan 13, 2020

Conversation

tknickman
Copy link
Contributor

Here is the PR for the data-testid rule. Let me know what you think! Happy to make any adjustments necessary.

Fixes #55

Copy link
Contributor

@emmenko emmenko left a comment

Choose a reason for hiding this comment

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

Thanks for your PR. I left some comments/questions to clarify a couple of things first.

README.md Outdated Show resolved Hide resolved
docs/rules/data-testid.md Outdated Show resolved Hide resolved
docs/rules/data-testid.md Outdated Show resolved Hide resolved
@tknickman
Copy link
Contributor Author

tknickman commented Dec 20, 2019

Thanks for the review! I made the requested changes here: c7aa60f

Let me know what ya'll think, happy to continue iterating on it 😄

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.

Sorry for reviewing this so late! Thanks for this PR, this looks really nice. Just one thing: what if I use a custom test id attr? For example, in one of the projects I work this is customized to data-test-id. Would be possible to update the rule to have the option to customize the attr? Thanks!!

@tknickman
Copy link
Contributor Author

@Belco90 no problem!. That's a great call, I totally forgot that was configurable now in RTL. I'll add that as a configuration option.

Belco90
Belco90 previously approved these changes Jan 13, 2020
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.

Awesome! This looks good to go 🚀

@tknickman
Copy link
Contributor Author

Forgot to update the docs, that commit is coming in 2 mins!

@Belco90 Belco90 dismissed their stale review January 13, 2020 16:07

problem with tests

@Belco90
Copy link
Member

Belco90 commented Jan 13, 2020

Also, the test coverage should be increased: https://travis-ci.org/Belco90/eslint-plugin-testing-library/jobs/636430039

I don't know why this wasn't blocking the PR, need to check it in a different ticket.

@tknickman
Copy link
Contributor Author

Coverage and docs updated, let me know if there's anything else I can help with!

@Belco90
Copy link
Member

Belco90 commented Jan 13, 2020

Brilliant, merging now for real.

@Belco90 Belco90 merged commit 6fd2e24 into testing-library:master Jan 13, 2020
@Belco90
Copy link
Member

Belco90 commented Jan 13, 2020

🎉 This PR is included in version 1.4.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@Belco90
Copy link
Member

Belco90 commented Jan 13, 2020

@all-contributors please add @tknickman for code, doc and test

@allcontributors
Copy link
Contributor

@Belco90

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

@tknickman
Copy link
Contributor Author

Thanks!

@tknickman tknickman deleted the data-testid_rule branch January 13, 2020 17:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rule to enforce consistent data-testid format
3 participants