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

Add markdown style guide to CONTRIBUTING #4027

Merged
merged 7 commits into from
Aug 22, 2024
Merged

Conversation

mikekistler
Copy link
Contributor

This PR adds a Markdown style guide to the CONTRIBUTING.md. I've lifted the bullets from @ralfhandl 's PRs on style fixup and added the .markdownlint.yaml file from @lornajane 's PRs for markdown updates.

There may be other things to add but I think this is a pretty good start and since we are already acting on these I thought we should make sure we have consensus and then capture that in the docs.

@mikekistler
Copy link
Contributor Author

We should add the rules for "property" vs "field" vs "keyword" in #4020

Copy link
Contributor

@ralfhandl ralfhandl left a comment

Choose a reason for hiding this comment

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

+1, some suggestions inline

CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
mikekistler and others added 2 commits August 15, 2024 08:30
Co-authored-by: Ralf Handl <ralf.handl@sap.com>
Co-authored-by: Ralf Handl <ralf.handl@sap.com>
ralfhandl
ralfhandl previously approved these changes Aug 15, 2024
@lornajane
Copy link
Contributor

In the TDC meeting we discussion that we should be using <span id="thing"></span> instead of the <a name="thing"></a> format for link anchors - it's been deprecated.

@handrews handrews added the editorial Wording and stylistic issues label Aug 15, 2024
@ralfhandl ralfhandl requested a review from a team August 16, 2024 07:50
ralfhandl
ralfhandl previously approved these changes Aug 16, 2024
@ralfhandl ralfhandl requested a review from a team August 16, 2024 13:33
@mikekistler
Copy link
Contributor Author

Can I merge an "editorial" PR with just one approval?

@ralfhandl
Copy link
Contributor

Unfortunately we haven't yet resolved

@ralfhandl
Copy link
Contributor

Btw., I forgot that you want to add .markdownlint.yaml in this PR in the project root.

In PRs #4040, #4041, and #4025 I add it in the versions folder, I can clean that up once this is merged.

Would you mind to update this PR and

npm i -D markdownlint-cli

so that we have the tool to interpret .markdownlint.yaml as a dev dependency?

ralfhandl
ralfhandl previously approved these changes Aug 20, 2024
@ralfhandl
Copy link
Contributor

Resolved conflict in package.json, refreshed package-lock.json

ralfhandl
ralfhandl previously approved these changes Aug 20, 2024
@handrews
Copy link
Member

@mikekistler I was intending to actually look over this by Thursday, but I have no objection to you merging it as we can always tweak it later.

@ralfhandl
Copy link
Contributor

Refreshed package-lock.json due to action errors in my fork

Copy link
Contributor

@lornajane lornajane left a comment

Choose a reason for hiding this comment

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

Thanks!

@lornajane lornajane merged commit 097ee45 into main Aug 22, 2024
4 checks passed
@lornajane lornajane deleted the markdown-style-guide branch August 22, 2024 12:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
editorial Wording and stylistic issues
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants