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

fix: typos and grammar #190

Merged
merged 4 commits into from
Sep 23, 2022
Merged

fix: typos and grammar #190

merged 4 commits into from
Sep 23, 2022

Conversation

jonas-jonas
Copy link
Member

Copy link
Contributor

@kevgo kevgo left a comment

Choose a reason for hiding this comment

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

These are really excellent changes, thanks @jonas-jonas! This is a good reminder for me to add the grammarly plugin back to VSCode. A few minor nitpicks about details below, then this is ready to 🚀.

Comment on lines 16 to 27
- [Contribute to Ory $PROJECT](#contribute-to-ory-project)
- [Introduction](#introduction)
- [FAQ](#faq)
- [How can I contribute?](#how-can-i-contribute)
- [Communication](#communication)
- [Contribute examples](#contribute-examples)
- [Contribute code](#contribute-code)
- [Contribute documentation](#contribute-documentation)
- [Disclosing vulnerabilities](#disclosing-vulnerabilities)
- [Code style](#code-style)
- [Working with forks](#working-with-forks)
- [Conduct](#conduct)
Copy link
Contributor

Choose a reason for hiding this comment

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

This shouldn't get updated in a pull request that just fixes typos. Can you please revert this part?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure how that happens, but I think it's prettier in VSCode?

It automatically does this when I save the file... So this might be correct, actually? Do you know where this is coming from?

templates/repository/common/CONTRIBUTING.md Outdated Show resolved Hide resolved
templates/repository/common/SECURITY.md Outdated Show resolved Hide resolved
@kevgo kevgo changed the title fix: typoes and grammar fix: typos and grammar Sep 22, 2022
Comment on lines 16 to 27
- [Contribute to Ory $PROJECT](#contribute-to-ory-project)
- [Introduction](#introduction)
- [FAQ](#faq)
- [How can I contribute?](#how-can-i-contribute)
- [Communication](#communication)
- [Contribute examples](#contribute-examples)
- [Contribute code](#contribute-code)
- [Contribute documentation](#contribute-documentation)
- [Disclosing vulnerabilities](#disclosing-vulnerabilities)
- [Code style](#code-style)
- [Working with forks](#working-with-forks)
- [Conduct](#conduct)
Copy link
Contributor

@kevgo kevgo Sep 22, 2022

Choose a reason for hiding this comment

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

This is still here. Please save the file without running Prettier by pressing ctrl-alt-s (on Linux or Windows). Not sure what the hotkey on macOS is.

Alternatively, you can stage (select to be committed) only certain parts of a document:

  • on the CLI: run git commit -p, then enter y if you want to commit the current part or n if you don't want to commit it
  • in VSCode: enter "Source Control" view, double-click to see the diff of your changes, highlight the changes to commit, right-click, and select "stage selected ranges". Git commits only the changes that are "staged". You can also un-stage changes by selecting and right-clicking them.

If it's easier, you can also revert only parts of a document (those which you don't want to commit). In the diff view, highlight the part to be reverted, right clicking, and choosing "revert selected ranges".

Copy link
Member Author

Choose a reason for hiding this comment

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

My point was, that we should check why prettier is doing this.
Because the next time we (or anyone else) touches this file, they will run into the same issue. Currently, it doesn't seem possible with doctoc (see thlorenz/doctoc#213) to ignore headings though, so I don't think there is another way of solving this for now.

Done. PTAL.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think Prettier is doing this since Prettier never changes content like this (it only formats it) and TOC are not even an official part of the Markdown spec. This is done by a much more invasive tool. I suspect a VSCode plugin like https://marketplace.visualstudio.com/items?itemName=yzhang.markdown-all-in-one here. That's at least the culprit in my setup. Super useful plugin btw.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ohh, right! That's true. I have it installed too...

Well, then we could add <!-- omit in toc --> (see [1] from their README) to the end of the heading # Contribute to Ory $PROJECT. I am not sure if that'll break anything, though. I tested it and at least it doesn't generate the heading into the TOC anymore. WDYT?

1:
image

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds promising. Since that's a separate issue and this branch is shipped already, do you want to give it a try in a separate feature branch?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure thing: #192

Copy link
Member

@vinckr vinckr left a comment

Choose a reason for hiding this comment

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

awesome, i did not realize we had so many typos in there 😅

@vinckr
Copy link
Member

vinckr commented Sep 23, 2022

test Expected — Waiting for status to be reported

Is this stuck or something?

@jonas-jonas
Copy link
Member Author

Is this stuck or something?

Yes, I believe so. Let's wait for @kevgo to approve.

Copy link
Contributor

@kevgo kevgo left a comment

Choose a reason for hiding this comment

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

This PR is a big step up for the developer experience. Such details shape the perception of how mature (and thereby trustworthy) our org and its processes are.

It keeps amazing me how many issues good automation finds even in very carefully human-written and human-reviewed content like the Markdown in this PR. Vincent and I have reviewed and improved on it so many times and still. The exact same thing applies to human-written source code. Modern test tools still find bugs even in decades-old Unix tools that are heavily battle tested and had thousands of eyeballs on them. To me this says something about how complementary the strengths and weaknesses of humans (creative/friendly/empathic but slow/sloppy/unreliable) vs machines (fast/exact/logical but unoriginal/uncreative).

Let's keep this high level of excellence by using all available tools (free and the best ones money can buy) like syntax and grammar checkers, readability checkers, linters, type checkers, test programs, etc.

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.

3 participants