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 with prettier #258

Merged

Conversation

harrisonhjones
Copy link
Contributor

@harrisonhjones harrisonhjones commented Mar 21, 2023

Description

Just a simple "let's format the markdown files" PR. Ran npx prettier --prose-wrap "always" -w *.md and committed the results.

Issues Resolved

None

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@harrisonhjones harrisonhjones marked this pull request as ready for review March 21, 2023 16:17
@harrisonhjones harrisonhjones force-pushed the pr-format-with-prettier branch from 7738563 to 5158651 Compare March 21, 2023 16:23
@harrisonhjones
Copy link
Contributor Author

Might be worth it to do this after merging in #259

Copy link
Member

@dblock dblock left a comment

Choose a reason for hiding this comment

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

We've been keeping Markdown files without the 80 column thing because it makes diffs unreadable - https://code.dblock.org/2021/06/07/to-wrap-or-not-to-wrap-in-markdown.html. Otherwise down with prettier for code?

@harrisonhjones
Copy link
Contributor Author

Got it. I'm fairly sure prettier can't help with Go (that's what go fmt is for) bit prettier can help with keeping markdown files consistent. I can re-run w/o prose-wrap to address your wrap concern.

- Ran `npx prettier -w *.md` and committed the results.

Signed-off-by: Harrison Jones <harrison@hhj.me>
@harrisonhjones harrisonhjones force-pushed the pr-format-with-prettier branch from 5158651 to f445893 Compare March 21, 2023 19:54
@harrisonhjones
Copy link
Contributor Author

I've rebased off of main and re-run w/o the prose wrap

@dblock
Copy link
Member

dblock commented Mar 22, 2023

Merging this, cause it looks ... prettier.

I'd like this to become part of CI. I also found opensearch-project/.github#66 that was opened a while ago asking for the same across the org. Maybe we can standardize this linter (or another) across the org? Care to comment on that issue and maybe try PRing something into .github?

@dblock dblock merged commit 5c62768 into opensearch-project:main Mar 22, 2023
@zethuman zethuman mentioned this pull request Mar 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants