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

Add Prettier to lint command #20674

Merged
merged 4 commits into from
May 19, 2023
Merged

Add Prettier to lint command #20674

merged 4 commits into from
May 19, 2023

Conversation

nschonni
Copy link
Contributor

These were removed from the Huskier PR, so resubmitting to review separately

@nschonni nschonni requested a review from a team as a code owner September 13, 2022 14:49
@nschonni
Copy link
Contributor Author

After this or with this one, we could add the prettier call to the Auto PR fix job

caugner
caugner approved these changes Sep 14, 2022
View reviewed changes
Copy link
Contributor

@caugner caugner left a comment

Choose a reason for hiding this comment

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

LGTM, but the only problem I see is that those "already fixed" files don't get auto-fixed via lint-staged yet, so the failing test may be a surprise for contributors.

@teoli2003
Copy link
Contributor

We want to discuss the plan to move forward with Prettier once everybody returns from the all-hand Mozilla trip (for some), and holidays (for others).

Meanwhile, we can fix content to match the output of Prettier, getting experience with the limitations of the tool (weirdness with inline elements, CDATA and MathML code "minimization", a few oddities with CSS, …), but we shouldn't move forward with automation before the plan has been discussed on mdn/mdn-community/discussions and agreed.

@nschonni
Copy link
Contributor Author

LGTM, but the only problem I see is that those "already fixed" files don't get auto-fixed via lint-staged yet, so the failing test may be a surprise for contributors.

When these folders are opted back in, lint-stage/husky will autofix any regressions going forward

@caugner
Copy link
Contributor

caugner commented Oct 24, 2022

@teoli2003 Just following up: Have you had a chance to discuss this?

@nschonni
Copy link
Contributor Author

Gave this a rebase, and updated the globs for the global job and scripts to run for prettier changes. Still green with the now opted in folders

@nschonni nschonni mentioned this pull request Nov 11, 2022
Merged
@github-actions
Copy link
Contributor

github-actions bot commented Dec 6, 2022

This pull request has merge conflicts that must be resolved before it can be merged.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 1, 2023

This pull request has merge conflicts that must be resolved before it can be merged.

@nschonni nschonni requested review from a team as code owners February 1, 2023 18:32
@nschonni nschonni requested review from teoli2003 and removed request for a team February 1, 2023 18:32
@github-actions github-actions bot added Content:HTTP HTTP docs Content:Other Any docs not covered by another "Content:" label and removed rebase needed 🚧 labels Feb 1, 2023
@nschonni
Copy link
Contributor Author

nschonni commented Feb 1, 2023

@queengooborg @bsmth noticed that #24046 landed. Maybe you're ready to look at this now

@github-actions
Copy link
Contributor

github-actions bot commented Feb 1, 2023 •
edited
Loading

Preview URLs (6 pages)
Flaws (1)

Note! 5 documents with no flaws that don't need to be listed. 🎉

URL: /en-US/docs/Web/CSS/CSS_Scroll_Snap/Basic_concepts
Title: Basic concepts of CSS Scroll Snap
Flaw count: 1

  • broken_links:
    • Can't resolve /en-US/docs/Web/CSS/CSS_Scroll_Snap/

(comment last updated: 2023-05-19 23:33:30)

bsmth
bsmth reviewed Feb 3, 2023
View reviewed changes
.prettierignore Outdated
!files/en-us/mdn/**/*.md
!files/en-us/web/http/**/*.md
!/*.md
!/files/en-us/games/**/*.md
Copy link
Member

Choose a reason for hiding this comment

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

What do you think about getting rid of the leading slash for these paths? e.g.

!files/en-us/games/**/*.md

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I kept if consistent because the / is needed for the root path, and the absolute gives an explicit match

Copy link
Member

Choose a reason for hiding this comment

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

OK good to know. If this is the preferred way, we should continue with that convention when adding other paths (note to self & @queengooborg).

bsmth
bsmth reviewed Feb 3, 2023
View reviewed changes
.prettierignore Outdated Show resolved Hide resolved
@github-actions github-actions bot added Content:CSS Cascading Style Sheets docs Content:WebAPI Web API docs labels Feb 26, 2023
@nschonni nschonni mentioned this pull request Mar 2, 2023
Closed
@Josh-Cena Josh-Cena changed the title chore: Opt-in cleaned folders and CI for Prettier Add Prettier to lint command Mar 30, 2023
@dipikabh dipikabh removed their request for review April 4, 2023 13:26
@github-actions github-actions bot added the merge conflicts 🚧 [PR only] label Apr 28, 2023
@github-actions
Copy link
Contributor

This pull request has merge conflicts that must be resolved before it can be merged.

@github-actions github-actions bot removed Content:WebAPI Web API docs Content:CSS Cascading Style Sheets docs merge conflicts 🚧 [PR only] labels Apr 28, 2023
@queengooborg queengooborg requested review from a team and Ryuno-Ki and removed request for a team May 19, 2023 23:25
@github-actions github-actions bot added Content:CSS Cascading Style Sheets docs Content:SVG SVG docs Content:WebAPI Web API docs labels May 19, 2023
queengooborg
queengooborg approved these changes May 19, 2023
View reviewed changes
Copy link
Collaborator

@queengooborg queengooborg left a comment

Choose a reason for hiding this comment

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

Since there was only six files left in need of formatting, I've gone ahead and just formatted them now so we can get this landed!

@queengooborg queengooborg merged commit 4bbb203 into mdn:main May 19, 2023
@nschonni nschonni deleted the prettier-ci branch May 19, 2023 23:45
@queengooborg queengooborg mentioned this pull request May 19, 2023
Closed
73 tasks
wbamberg added a commit to wbamberg/content that referenced this pull request May 20, 2023
@wbamberg
Merge remote-tracking branch 'origin/update-beforeinstallpromptevent'…
6f3a413 
… into update-beforeinstallpromptevent

* origin/update-beforeinstallpromptevent:
  Add Prettier to lint command (mdn#20674)
  Remove SVG <discard> docs (mdn#26856)
  Remove docs for <applet> (mdn#26850)
  refactor: migrate media.prod.mdn.mozit.cloud URLs (mdn#26809)
  chore: Remove embedlivesample macro calls with a fourth parameter (mdn#26816)
  Bump @mdn/yari from 2.21.0 to 2.22.0 (mdn#26858)
  WebTransport API (mdn#26529)
  Fix issue 26739: correct beforeunload situation wrt bfcache (mdn#26819)
  add missing colon (mdn#26846)
  Replace reference to time domain with amplitude domain in `AnalyserNode .fftSize` (mdn#26840)
  Fix issue 26048: document security requirements for opening and focus… (mdn#26838)
  Change all CSS spec URLs back to drafts.csswg.org (mdn#26833)
  Synchronize with BCD v5.2.58 (mdn#26829)
  Rename nonexistent globalScope to globalThis in service worker event examples (mdn#26808)
  Small typo fix (mdn#26822)
  removing scrollbar (mdn#26813)
  Fix typo (mdn#26826)
@bsmth bsmth mentioned this pull request May 31, 2023
Merged
wbamberg added a commit to wbamberg/content that referenced this pull request Jun 1, 2023
@wbamberg
Merge remote-tracking branch 'origin/pwa-main-page' into pwa-main-page
c953f68 
* origin/pwa-main-page: (86 commits)
  Add Prettier to lint command (mdn#20674)
  Remove SVG <discard> docs (mdn#26856)
  Remove docs for <applet> (mdn#26850)
  refactor: migrate media.prod.mdn.mozit.cloud URLs (mdn#26809)
  chore: Remove embedlivesample macro calls with a fourth parameter (mdn#26816)
  Bump @mdn/yari from 2.21.0 to 2.22.0 (mdn#26858)
  WebTransport API (mdn#26529)
  Fix issue 26739: correct beforeunload situation wrt bfcache (mdn#26819)
  add missing colon (mdn#26846)
  Replace reference to time domain with amplitude domain in `AnalyserNode .fftSize` (mdn#26840)
  Fix issue 26048: document security requirements for opening and focus… (mdn#26838)
  Change all CSS spec URLs back to drafts.csswg.org (mdn#26833)
  Synchronize with BCD v5.2.58 (mdn#26829)
  Rename nonexistent globalScope to globalThis in service worker event examples (mdn#26808)
  Small typo fix (mdn#26822)
  removing scrollbar (mdn#26813)
  Fix typo (mdn#26826)
  docs(:not()): Update the syntax and example titles (mdn#26814)
  Add a Performance guide for Server Timing (mdn#25575)
  Fix a summary (mdn#26815)
  ...
@yarusome yarusome mentioned this pull request Jun 30, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bsmth bsmth bsmth left review comments

@queengooborg queengooborg queengooborg approved these changes

@caugner caugner caugner approved these changes

@Ryuno-Ki Ryuno-Ki Awaiting requested review from Ryuno-Ki

Assignees
No one assigned
Labels
Content:CSS Cascading Style Sheets docs Content:SVG SVG docs Content:WebAPI Web API docs
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@nschonni @teoli2003 @caugner @queengooborg @bsmth