Skip to content

docs: Fix container destructuring #805

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

Merged
merged 1 commit into from
Apr 27, 2021

Conversation

ErfanMirzapour
Copy link
Contributor

@ErfanMirzapour ErfanMirzapour commented Apr 21, 2021

Doc page: (q4)
https://testing-library.com/docs/dom-testing-library/faq

The suggested plugin is no longer maintained.
Just changed it with newer/active one.

+Fix a syntax error in q5.

@eps1lon
Copy link
Member

eps1lon commented Apr 21, 2021

The suggested plugin is no longer maintained.

Could you give some context to this statement? I could not find anything supporting that claim.

@ErfanMirzapour
Copy link
Contributor Author

@eps1lon Check npm last publish.

@eps1lon
Copy link
Member

eps1lon commented Apr 22, 2021

@eps1lon Check npm last publish.

Which is 0.3. Neither 0.3 nor https://www.npmjs.com/package/babel-plugin-react-remove-properties/ are marked as deprecated. Could you be more specific why you think that package is no longer maintained?

@ErfanMirzapour
Copy link
Contributor Author

ErfanMirzapour commented Apr 22, 2021

Which is 0.3. Neither 0.3 nor https://www.npmjs.com/package/babel-plugin-react-remove-properties/ are marked as deprecated. Could you be more specific why you think that package is no longer maintained?

I meant the last publish date which is March 2019!
I didn't say it's deprecated. It just could be a little bit frightening to use a plugin that hasn't been updated for 2 years!

Also supporting active packages is encouraging for their maintainers. That's just my opinion!

@eps1lon
Copy link
Member

eps1lon commented Apr 23, 2021

It just could be a little bit frightening to use a plugin that hasn't been updated for 2 years!

It didn't need one for over 2 years. That's a good sign. I looked at the repo and it has a single open issue. No open PRs. That's one of the cleanest states I've seen a repo in that didn't have an update in 2 years.

Also supporting active packages is encouraging for their maintainers.

It also encourages maintainers to constantly re-publish their package just to stay relevant. Let's relieve maintainers of some pressure. I'm just speaking for myself here but I'm no looking forward to having all my packages replaced just because I didn't update needlessly.

I meant the last publish date which is March 2019!
I didn't say it's deprecated.

"no longer maintained" and "deprecated" are very close concepts but I understand that this was putting words in your mouth. Still, not having received an update is not sufficient evidence that it's no longer maintained. Especially if said package didn't need maintenance. And I work with the author on other projects so I know he's still active.

@ErfanMirzapour
Copy link
Contributor Author

It didn't need one for over 2 years. That's a good sign. I looked at the repo and it has a single open issue. No open PRs. That's one of the cleanest states I've seen a repo in that didn't have an update in 2 years.

That's right.

Especially if said package didn't need maintenance. And I work with the author on other projects so I know he's still active.

Cool, I didn't know that.
I re-wrote branch history and removed the plugin commit.

Copy link
Member

@nickserv nickserv left a comment

Choose a reason for hiding this comment

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

This is a good fix, but we should probably rename the commit on merge

@eps1lon
Copy link
Member

eps1lon commented Apr 27, 2021

This is a good fix, but we should probably rename the commit on merge

@nickmccurdy Could you clarify why this is a good fix in your opinion in light of my comments in this conversation?

Nevermind, the PR was changed.

@eps1lon eps1lon changed the title docs: Change suggested plugin for removing data-testid docs: Fix container destructuring Apr 27, 2021
@eps1lon eps1lon merged commit c160fcd into testing-library:master Apr 27, 2021
@eps1lon
Copy link
Member

eps1lon commented Apr 27, 2021

@ErfanMirzapour Thanks!

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