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

Format markdown with prettier and prepare SVG graphs for hooks #4206

Merged
merged 2 commits into from Aug 14, 2021

Conversation

@lukastaegert
Copy link
Member

@lukastaegert lukastaegert commented Aug 14, 2021

This PR contains:

  • bugfix
  • feature
  • refactor
  • documentation
  • other

Are tests included?

  • yes (bugfixes and features will not be merged without tests)
  • no

Breaking Changes?

  • yes (breaking changes will not be merged unless absolutely necessary)
  • no

List any relevant issue numbers:

Description

This will replace markdownlint with prettier for formatting markdown files, which looks much better also with respect to code samples.

It also adds two mermaid definition files that can be used to generate graphs for the build and output plugin hooks. Moreover, there is an HTML snippet to generate a legend for those files.
In the documentation itself, there are comments

<!-- html:hooks-legend.html -->
<!-- mermaid:build-hooks.mmd -->

That will be used by a forthcoming website update to actually replace them with the rendered graphs.

@github-actions
Copy link

@github-actions github-actions bot commented Aug 14, 2021

Thank you for your contribution! ❤️

You can try out this pull request locally by installing Rollup via

npm install rollup/rollup#prettier-markdown

or load it into the REPL:
https://rollupjs.org/repl/?pr=4206

@codecov
Copy link

@codecov codecov bot commented Aug 14, 2021

Codecov Report

Merging #4206 (50b44ad) into master (2f74020) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #4206   +/-   ##
=======================================
  Coverage   98.37%   98.37%           
=======================================
  Files         202      202           
  Lines        7248     7248           
  Branches     2118     2118           
=======================================
  Hits         7130     7130           
  Misses         58       58           
  Partials       60       60           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2f74020...50b44ad. Read the comment docs.

@lukastaegert
Copy link
Member Author

@lukastaegert lukastaegert commented Aug 14, 2021

Will merge this because I need the comments on master to fine-tune rollup/rollupjs.org#230

@lukastaegert lukastaegert merged commit 69c099c into master Aug 14, 2021
9 checks passed
@lukastaegert lukastaegert deleted the prettier-markdown branch Aug 14, 2021
@shellscape
Copy link
Contributor

@shellscape shellscape commented Aug 14, 2021

FWIW the eslint+prettier config and setup in the plugins repo is very mature now. Might be worth a look.

@lukastaegert
Copy link
Member Author

@lukastaegert lukastaegert commented Aug 14, 2021

True, but I was under the impression that even with plugins, ESLint will only format code blocks inside markdown files while prettier also formats the surrounding content, which is something I liked very much. But as I did not check, this assumption might be wrong.

@shellscape
Copy link
Contributor

@shellscape shellscape commented Aug 14, 2021

The solution in plugins does both :)

@lukastaegert
Copy link
Member Author

@lukastaegert lukastaegert commented Aug 14, 2021

But it looks you are also using just bare prettier for markdown: https://github.com/rollup/plugins/blob/6463b9635e20b8425599f14005b9efdd8f94ecd6/package.json#L7

@shellscape
Copy link
Contributor

@shellscape shellscape commented Aug 14, 2021

It's a supplementary step. You don't have to take my word for it, but it runs like smooth butter over there. I've implemented the same setup for two companies now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants