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

chore: delete stale snapshots #3062

Merged
merged 3 commits into from
Jun 5, 2024
Merged

chore: delete stale snapshots #3062

merged 3 commits into from
Jun 5, 2024

Conversation

siosio34
Copy link
Contributor

@siosio34 siosio34 commented Jun 5, 2024

Summary

hello. I am one of the users who watches biome closely because I feel frustrated with the speed of using eslint.

recently, the trailingComma property has been changed to trailingCommas, but it seems to remain as trailingComma in some documents.

so, I felt it was necessary to revise the document so that others would not have to go through the same trial and error as me.

If the modification is incorrect, you may make additional corrections.

and thank you for making a great product.

Test Plan

@github-actions github-actions bot added A-CLI Area: CLI A-Project Area: project A-Formatter Area: formatter L-JavaScript Language: JavaScript and super languages labels Jun 5, 2024
Copy link
Member

@Sec-ant Sec-ant left a comment

Choose a reason for hiding this comment

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

Hello, thanks for helping catch the descrepencies. However there're two kinds of changes in this PR that need to be reverted:

  1. Prettier uses the singular form trailingComma as its option name, we shouldn't change that.
  2. We have some tests to check that we can handle the deprecated option name tailingComma, so we shouldn't change them.

The changes in the test snapshots seem expected. However I'm not sure why they weren't catched earlier? Did you run the tests to update the snapshots or did you just replace them by hand?

@siosio34
Copy link
Contributor Author

siosio34 commented Jun 5, 2024

Hello, thanks for helping catch the descrepencies. However there're two kinds of changes in this PR that need to be reverted:

  1. Prettier uses the singular form trailingComma as its option name, we shouldn't change that.
  2. We have some tests to check that we can handle the deprecated option name tailingComma, so we shouldn't change them.

The changes in the test snapshots seem expected. However I'm not sure why they weren't catched earlier? Did you run the tests to update the snapshots or did you just replace them by hand?

I did not understand the project well and updated it manually. @Sec-ant

@github-actions github-actions bot removed A-Project Area: project A-Formatter Area: formatter labels Jun 5, 2024
@siosio34
Copy link
Contributor Author

siosio34 commented Jun 5, 2024

I'm sorry that I checked incorrectly and caused you trouble when you could easily fix it. I have corrected everything you mentioned, and if there is anything else that needs to be corrected, please let me know.

d6d9654

@Sec-ant
Copy link
Member

Sec-ant commented Jun 5, 2024

I did not understand the project well and updated it manually.

I see. Test snapshots reflect the behavior of the source code and test cases, they are meant to catch unexpected regressions. So they should only be mutated when the source code or test cases change their behavior, and we shouldn't change them manually.

That being said, the snapshots updated by this PR are actually stale snapshots that should be deleted. I'll just edit in your branch. Thanks for catching them.

@Sec-ant Sec-ant changed the title docs: trailingComma to trailingCommas chore: delete stale snapshots Jun 5, 2024
@Sec-ant Sec-ant merged commit 6075493 into biomejs:main Jun 5, 2024
11 checks passed
@Conaclos Conaclos added the A-Changelog Area: changelog label Sep 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Changelog Area: changelog A-CLI Area: CLI L-JavaScript Language: JavaScript and super languages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants