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

Remove some, with Prettier 2.3.0, unnecessary // prettier-ignore comments #13401

Merged
merged 1 commit into from
May 21, 2021

Conversation

Snuffleupagus
Copy link
Collaborator

To get the maximum benefit from something like Prettier, you obviously don't want to disable the automatic formatting unless absolutely necessary. When we added Prettier there were a number of cases, mostly involving larger Arrays, which required disabling of the automatic formatting for overall readability and/or to not break inline comments.

With changes in Prettier version 2.3.0, see the release notes, there's now better formatting support for Arrays containing only numbers. Hence we can now remove a number of // prettier-ignore comments, and thus get the benefit of automatic formatting in (slightly) more of the code-base.

…comments

To get the maximum benefit from something like Prettier, you obviously don't want to disable the automatic formatting unless absolutely necessary. When we added Prettier there were a number of cases, mostly involving larger Arrays, which required disabling of the automatic formatting for overall readability and/or to not break inline comments.

With changes in Prettier version `2.3.0`, see [the release notes](https://prettier.io/blog/2021/05/09/2.3.0.html#concise-formatting-of-number-only-arrays-10106httpsgithubcomprettierprettierpull10106-10160httpsgithubcomprettierprettierpull10160-by-thorn0httpsgithubcomthorn0), there's now better formatting support for Arrays containing only numbers. Hence we can now remove a number of `// prettier-ignore` comments, and thus get the benefit of automatic formatting in (slightly) more of the code-base.
@Snuffleupagus
Copy link
Collaborator Author

/botio test

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Received

Command cmd_test from @Snuffleupagus received. Current queue size: 0

Live output at: http://54.67.70.0:8877/66707f5d0df6ccc/output.txt

@pdfjsbot
Copy link

From: Bot.io (Windows)


Received

Command cmd_test from @Snuffleupagus received. Current queue size: 0

Live output at: http://3.101.106.178:8877/27494848c6af72e/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Failed

Full output at http://54.67.70.0:8877/66707f5d0df6ccc/output.txt

Total script time: 26.36 mins

  • Font tests: Passed
  • Unit tests: Passed
  • Integration Tests: Passed
  • Regression tests: FAILED

Image differences available at: http://54.67.70.0:8877/66707f5d0df6ccc/reftest-analyzer.html#web=eq.log

@pdfjsbot
Copy link

From: Bot.io (Windows)


Failed

Full output at http://3.101.106.178:8877/27494848c6af72e/output.txt

Total script time: 30.08 mins

  • Font tests: Passed
  • Unit tests: Passed
  • Integration Tests: FAILED
  • Regression tests: FAILED

Image differences available at: http://3.101.106.178:8877/27494848c6af72e/reftest-analyzer.html#web=eq.log

@timvandermeij timvandermeij merged commit 7d9a830 into mozilla:master May 21, 2021
@timvandermeij
Copy link
Contributor

Thank you for doing this!

@Snuffleupagus Snuffleupagus deleted the rm-some-prettier-ignore branch May 21, 2021 20:52
@Snuffleupagus
Copy link
Collaborator Author

@timvandermeij As always, thank you for landing my patches :-)


Somewhat unrelated, but is it too soon to start thinking about creating a new release now?

Historically we've probably, more than once, gone a bit too long between releases at times. Having releases happening around e.g. every 6-8 weeks[1] would mean that users get access to bug fixes and new features much quicker (since many users seem to prefer the stable versions).
Also, in recent months the development pace has picked up nicely, and having e.g. fewer new features and overall changes in each release might help reduce (possible) bugs in any one release!?


[1] Note how the release cycle of e.g. Firefox is even shorter nowadays, see https://wiki.mozilla.org/Release_Management/Calendar

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.

3 participants