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

[doc] Fix typo and formatting in CONTRIBUTING.md #11381

Merged
merged 2 commits into from
Jan 17, 2024
Merged

[doc] Fix typo and formatting in CONTRIBUTING.md #11381

merged 2 commits into from
Jan 17, 2024

Conversation

Architector4
Copy link
Contributor

Objective

Issue: There is a typo in CONTRIBUTING.md ("then" used in place of "them"). There is also an inconsistency of usage of periods at ends of items in lists, and one section is written with non-breaking spaces without good reason.

Solution

Fix the aforementioned typo and consistency issues.

# Objective

Issue: There is a typo in `CONTRIBUTING.md` ("then" used in place of "them"). There is also an inconsistency of usage of periods at ends of items in lists, and one section is written with non-breaking spaces without good reason.

## Solution

Fix the aforementioned typo and consistency issues.
Copy link
Contributor

Welcome, new contributor!

Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨

@Architector4
Copy link
Contributor Author

Architector4 commented Jan 17, 2024

Whoops, forgot to mention making items of the first list in the document start with an uppercase letter, and removal of one probably insignificant newline and two trailing spaces at the end of one line.

Usage of newlines seems sporadic in the document in general, but I felt it would make this a bit more annoying of a commit to deal with, and so decided to not do anything with that lol

@mockersf mockersf added C-Docs An addition or correction to our documentation S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it labels Jan 17, 2024
Copy link
Contributor

@Jondolf Jondolf left a comment

Choose a reason for hiding this comment

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

Some (maybe subjective) punctuation nits, otherwise looks good to me :)

Comment on lines +86 to +91
* A: Area (e.g. A-Animation, A-ECS, A-Rendering).
* C: Category (e.g. C-Breaking-Change, C-Code-Quality, C-Docs).
* D: Difficulty (e.g. D-Complex, D-Good-First-Issue).
* O: Operating System (e.g. O-Linux, O-Web, O-Windows).
* P: Priority (e.g. P-Critical, P-High).
* S: Status (e.g. S-Blocked, S-Controversial, S-Needs-Design).
Copy link
Contributor

Choose a reason for hiding this comment

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

I wouldn't use periods here. Bulleted list items should generally use periods if they either contain a full sentence or complete the introductory stem sentence (the sentence before ":"), which is not the case here. Stylistically, ")." also doesn't look as clean in my opinion.

Comment on lines +81 to +82
* How controversial are the design decisions.
* How complex is the implementation.
Copy link
Contributor

Choose a reason for hiding this comment

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

These are questions, so if we want punctuation here, the sentences should end with a question mark.

Copy link
Contributor

Choose a reason for hiding this comment

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

Or rephrase as e.g. How complex the implementation is.

CONTRIBUTING.md Outdated Show resolved Hide resolved
Co-authored-by: Rob Parrett <robparrett@gmail.com>
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Jan 17, 2024
Merged via the queue into bevyengine:main with commit 8db4723 Jan 17, 2024
22 checks passed
@Architector4
Copy link
Contributor Author

Oop-

I'd have fixed the above too, but was asleep. Ah well, at least I hope I made somewhat of an improvement lol

@Architector4 Architector4 deleted the patch-1 branch January 18, 2024 02:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-Docs An addition or correction to our documentation S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants