Skip to content

Adding "how to create new page" to CONTRIBUTING.md - #1294

Merged
TMoMoreau merged 16 commits into
mainfrom
add/how-to-create-new-page
Dec 6, 2022
Merged

TMoMoreau merged 16 commits into
mainfrom
add/how-to-create-new-page

Conversation

@TMoMoreau

Copy link
Copy Markdown
Contributor

This PR is in reference to issue #1264

@TMoMoreau

TMoMoreau commented Oct 6, 2022 •

Copy link
Copy Markdown
Contributor Author

@johnnymatthews @ElPaisano I may still want to add more screenshots or go into more detail about certain things. Let me know what you think.

Comment thread CONTRIBUTING.md Outdated
Comment thread CONTRIBUTING.md Outdated
Comment thread CONTRIBUTING.md Outdated
@ElPaisano

ElPaisano commented Oct 7, 2022 •

Copy link
Copy Markdown
Contributor

@TMoMoreau made some general comments above, might be easier to just talk through them real quick

Comment thread CONTRIBUTING.md Outdated

@ElPaisano ElPaisano left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Made a first pass, I can take another look in a bit or after you push this - I don't want to be a blocker to getting it to prod

Comment thread CONTRIBUTING.md Outdated
Comment thread CONTRIBUTING.md Outdated
Comment thread CONTRIBUTING.md Outdated
Comment thread CONTRIBUTING.md Outdated
Comment thread CONTRIBUTING.md Outdated
Comment thread CONTRIBUTING.md Outdated
Comment thread CONTRIBUTING.md Outdated
Comment thread CONTRIBUTING.md Outdated
Comment thread CONTRIBUTING.md Outdated
Comment thread CONTRIBUTING.md Outdated
Comment thread CONTRIBUTING.md Outdated
Comment thread CONTRIBUTING.md Outdated
Comment thread CONTRIBUTING.md Outdated
Comment thread CONTRIBUTING.md Outdated
Comment thread CONTRIBUTING.md Outdated
Comment thread CONTRIBUTING.md Outdated
@ElPaisano

Copy link
Copy Markdown
Contributor

General comment:
The vale linter is showing a Flesch Reading Ease score of 68.2, which is pretty good. According to that metric, it means that the document is "easily understood by 13- to 15-year-old students". However, the vale target is 70, which indicates "fairly easy to read".

The score is a function of:

  • the average length of your sentences (measured by the number of words)
  • the average number of syllables per word

I ran markdownlint which, among other things, checks if line length is greater than 80. (this is configurable). I saw a lot of output like:

projects/testFiles/CONTRIBUTING.md:126:81 MD013/line-length Line length [Expected: 80; Actual: 282]
projects/testFiles/CONTRIBUTING.md:131:81 MD013/line-length Line length [Expected: 80; Actual: 107]
projects/testFiles/CONTRIBUTING.md:133:81 MD013/line-length Line length [Expected: 80; Actual: 125]
projects/testFiles/CONTRIBUTING.md:137:81 MD013/line-length Line length [Expected: 80; Actual: 419]
projects/testFiles/CONTRIBUTING.md:139:81 MD013/line-length Line length [Expected: 80; Actual: 208]
projects/testFiles/CONTRIBUTING.md:145:81 MD013/line-length Line length [Expected: 80; Actual: 184]

So, just something to keep in mind. If possible, keep sentences as short as possible, as it will make the overall reading experience easier for most readers. This will also bring the readability score up.

@ElPaisano

Copy link
Copy Markdown
Contributor

markdown-link-check came back clean 💯

@ElPaisano

Copy link
Copy Markdown
Contributor

Noticed a few other errors that aren't in this PR scope but are in CONTRIBUTING.md. Might be good to fix these here.

Line 79: "There're". This should be "There are"
Line 27: "Aquire". This should be "Acquire"

@TMoMoreau

Copy link
Copy Markdown
Contributor Author

Line 79: "There're". This should be "There are"

Fixed this because it's weird. I do want it to be known though, that this is technically a grammatically correct contraction.

@ElPaisano ElPaisano closed this Nov 17, 2022
@TMoMoreau TMoMoreau reopened this Nov 17, 2022

@johnnymatthews johnnymatthews left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Made a couple of suggestions, but looks great apart from those!

Comment thread CONTRIBUTING.md Outdated
Comment thread CONTRIBUTING.md Outdated
Comment thread CONTRIBUTING.md Outdated
Comment thread CONTRIBUTING.md Outdated
Comment thread CONTRIBUTING.md Outdated
@ElPaisano

Copy link
Copy Markdown
Contributor

Glanced over it, looks solid to me 👍

@salmad3 salmad3 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice @TMoMoreau! Added suggestions.

Comment thread CONTRIBUTING.md Outdated
Comment thread CONTRIBUTING.md Outdated
Comment thread CONTRIBUTING.md Outdated
Comment thread CONTRIBUTING.md Outdated
Comment thread CONTRIBUTING.md
Comment thread CONTRIBUTING.md Outdated
Comment thread CONTRIBUTING.md Outdated
Comment thread CONTRIBUTING.md Outdated
Comment thread CONTRIBUTING.md Outdated
Comment thread CONTRIBUTING.md

@johnnymatthews johnnymatthews left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good! Let's get this merged.

@filecorgi

Copy link
Copy Markdown
Contributor
  • Image optimization came back clean!
  • Vuepress build was successful!

@TMoMoreau

Copy link
Copy Markdown
Contributor Author

Looks like I need @DannyS03 to approve as well to get the "changes requested" thing green.

@TMoMoreau
TMoMoreau merged commit 3bc07b9 into main Dec 6, 2022
@TMoMoreau
TMoMoreau deleted the add/how-to-create-new-page branch December 6, 2022 16:20
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.

5 participants