Skip to content

🏗📖 Auto-format all markdown files in ampproject/amphtml with Prettier#25182

Merged
rsimha merged 7 commits intoampproject:masterfrom
rsimha:2019-10-18-PrettifyMarkdown
Oct 24, 2019
Merged

🏗📖 Auto-format all markdown files in ampproject/amphtml with Prettier#25182
rsimha merged 7 commits intoampproject:masterfrom
rsimha:2019-10-18-PrettifyMarkdown

Conversation

@rsimha
Copy link
Copy Markdown
Contributor

@rsimha rsimha commented Oct 21, 2019 •
edited
Loading

The AMP Project already uses Prettier to auto-format JS files and several non-JS files across the repo. This PR adds documentation files to the mix.

Highlights:

  • Includes .md files in the list of files that are checked by Prettier
  • Auto-formats all .md files in the repo by running gulp prettier --fix
  • Skips the broken links check for PRs that modify a large number of .md files
  • Prints suggested fixes when formatting errors are detected
  • Exempts GitHub issue templates (they use a custom header that's incompatible with Prettier)
  • Make extension generator (gulp make-extension) resilient to markdown file auto-fixing

Notes:

  • Most of these changes do not affect the rendered HTML. Exceptions are:
    • Attribute lists are changed to all-in-one-line if they all fit on one line
      image
    • Attribute lists are changed to one-per-line if they don't all fit on one line
      image
  • Once this PR is merged, .md files will have to satisfy Prettier's formatting requirements
  • A workflow to make your IDE auto-fix .md files on save is available here
  • An alternative is to manually run gulp prettify --local_changes --fix

Example of a failing check:

Fixes #25148

@rsimha rsimha mentioned this pull request Oct 21, 2019
Closed
@rsimha rsimha requested a review from cramforce October 21, 2019 23:53
@rsimha
Copy link
Copy Markdown
Contributor Author

rsimha commented Oct 22, 2019 •
edited
Loading

The link checker is reporting several seemingly broken links in our documentation ☹️ On taking a closer look, most of these failures are due to requests to amp.dev timing out, probably because of the sheer number of requests being made by the checker.

I don't think we should be running the broken links check for PRs that modify a large number (> 20) of markdown files, so I've skipped it for large PRs. The check will continue to run when a smaller number of files are touched.

@rsimha
Copy link
Copy Markdown
Contributor Author

rsimha commented Oct 22, 2019 •
edited
Loading

I sanity checked several of the changes in this PR, and found no visible changes to the rendered HTML in most cases. This is now ready for review.

@cramforce cramforce removed their request for review October 22, 2019 19:55
@ampproject ampproject deleted a comment from codecov bot Oct 23, 2019
@rsimha
Copy link
Copy Markdown
Contributor Author

rsimha commented Oct 23, 2019

Rebased and added a function to print suggested fixes for badly formatted .md files.

mrjoro
mrjoro reviewed Oct 23, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@mrjoro mrjoro left a comment

Choose a reason for hiding this comment

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

overall changes look fine (with a few comments) though I admit to not reviewing all 483 files in detail :)

@rsimha
Copy link
Copy Markdown
Contributor Author

rsimha commented Oct 23, 2019

Rebased, reverted changes to issue template headers, and exempted them from future checks.

@rsimha rsimha requested a review from mrjoro October 24, 2019 16:20
This was referenced Oct 24, 2019
Merged
Merged
@rsimha
Copy link
Copy Markdown
Contributor Author

rsimha commented Oct 24, 2019 •
edited
Loading

Some final notes:

  • Self-reviewed every file in contributing/, and most files in ads/, extensions/, etc.
  • Found and fixed a handful of existing bugs that were exposed by auto-formatting (tabs instead of spaces, bad formatting, etc.)
  • Added prettier-ignore overrides where required
  • Verified that the majority of .md files have no visible changes when rendered
  • Tested VSCode auto-fix-on-save workflow for .md files
  • Tested gulp prettify with --fix, --files, and --local_changes

Merging this now 🤞

@rsimha rsimha merged commit e33dd10 into ampproject:master Oct 24, 2019
@rsimha rsimha deleted the 2019-10-18-PrettifyMarkdown branch October 24, 2019 19:40
@rsimha rsimha mentioned this pull request Oct 25, 2019
Merged
micajuine-ho pushed a commit to micajuine-ho/amphtml that referenced this pull request Dec 27, 2019
@rsimha
🏗📖 Auto-format all markdown files in ampproject/amphtml wi… (amppro…
adee6fc 
…ject#25182)
@rsimha rsimha mentioned this pull request Jan 16, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mrjoro mrjoro mrjoro approved these changes

@pbakaus pbakaus Awaiting requested review from pbakaus

@jeffjose jeffjose Awaiting requested review from jeffjose

@sebastianbenz sebastianbenz Awaiting requested review from sebastianbenz

@newmuis newmuis Awaiting requested review from newmuis

@CrystalOnScript CrystalOnScript Awaiting requested review from CrystalOnScript

Assignees

No one assigned

Labels

cla: yes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Shall we auto-format our .md files with Prettier?

3 participants

@rsimha @mrjoro @googlebot