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

Docs: Fix broken links and typos, propose clarity improvements #1218

Closed
wants to merge 121 commits into from

Conversation

IAmHopp
Copy link
Contributor

@IAmHopp IAmHopp commented Aug 7, 2018

Pull request checklist

Make sure you:

For non-trivial changes, please make sure you also:

  • Added/Updated related documentation.
  • Added/Updated related tests.

Short description of the change(s)

Since there were no objections in #1216, I'm going through each hint doc and checking for typos and suggesting minor changes for improved readability. If there are any objections, please let me know and I'll abandon the PR.

Files to review:

  • hint-amp-validator
  • hint-apple-touch-icons
  • hint-axe
  • hint-babel-config
  • hint-babel-config/is-valid
  • hint-content-type
  • hint-disown-opener
  • hint-highest-available-document-mode
  • hint-html-checker
  • hint-http-cache
  • hint-http-compression
  • hint-https-only
  • hint-image-optimization-cloudinary
  • hint-manifest-app-name
  • hint-manifest-exists
  • hint-manifest-file-extension
  • hint-manifest-is-valid
  • hint-meta-charset-utf-8
  • hint-meta-theme-color
  • hint-meta-viewport
  • hint-minified-js
  • hint-no-bom
  • hint-no-broken-links
  • hint-no-disallowed-headers
  • hint-no-friendly-error-pages
  • hint-no-html-only-headers
  • hint-no-http-redirects -- See Docs: Fix typos in hint-no-http-redirects #1216.
  • hint-no-protocol-relative-urls
  • hint-no-p3p
  • hint-no-vulnerable-javascript-libraries
  • hint-performance-budget
  • hint-sri -- See Docs: Fix typos in hint-sri #1210.
  • hint-ssllabs
  • hint-strict-transport-security
  • hint-stylesheet-limits
  • hint-typescript-config
  • hint-typescript-config/consistent-casing
  • hint-typescript-config/import-helpers
  • hint-typescript-config/is-valid
  • hint-typescript-config/no-comments
  • hint-typescript-config/strict
  • hint-typescript-config/target
  • hint-validate-set-cookie-header -- See Docs: Fix typo in validate-set-cookie-header #1209.
  • hint-webpack-config
  • hint-webpack-config/config-exists
  • hint-webpack-config/is-installed
  • hint-webpack-config/is-valid
  • hint-webpack-config/module-esnext-javascript
  • hint-webpack-config/modules-false-babel
  • hint-webpack-config/no-devtool-in-prod
  • hint-x-content-type-options

Once done:

  • Additional passthrough [specific for broken links]
  • Final passthrough
  • Check files changed -- Vast majority of commits were merged ahead of time yesterday, so this wouldn't be valuable anymore.

@IAmHopp IAmHopp force-pushed the patch-1 branch 2 times, most recently from 9ffb281 to 2a19461 Compare August 7, 2018 18:37
@alrra
Copy link
Contributor

alrra commented Aug 7, 2018

@IAmHopp Thank you so much for doing this! 🎉 Let us know if we can help you with anything!

@IAmHopp
Copy link
Contributor Author

IAmHopp commented Aug 7, 2018

Sure thing! Appreciate the support.

I'll continue tomorrow. With any luck I won't swallow a bunch of commits accidently as I did today.

@@ -2,7 +2,7 @@

## Why is this important?

To avoid problems in your project, the webpack configuration has to be valid.
To avoid problems in your project, the webpack configuration must be valid.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm going to change this to needs to

@@ -119,7 +119,7 @@ The possible values and the associated speeds for `connectionType` are:
| Edge | 240 Kbps | 200 Kbps | 840ms |
| Dial | 49 Kbps | 30 Kbps | 120ms |

`loadTime` has to be a number greater than `1` and indicates the time in
`loadTime` must be a number greater than `1` and indicates the time in
Copy link
Contributor

@alrra alrra Aug 8, 2018

Choose a reason for hiding this comment

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

needs to

@@ -27,7 +27,7 @@ This hint checks that a website correctly uses SRI, more specifically:

* All the downloaded resources by an `<script>` or `<link rel="stylesheet">`
have an `integrity` attribute.
* [The `integrity` attribute has to be valid][sri format]. I.e.: it should
* [The `integrity` attribute must be valid][sri format]. I.e.: it should
Copy link
Contributor

@alrra alrra Aug 8, 2018

Choose a reason for hiding this comment

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

needs to

@molant
Copy link
Member

molant commented Aug 8, 2018

If not, any idea what's up with that out-of-place "i'?

Also the new section in the link is Accessibility is Important for Individuals, Businesses, Society in case we want a more up to date quote.

Thanks a lot for your work @IAmHopp !

@alrra
Copy link
Contributor

alrra commented Aug 8, 2018

@IAmHopp Your changes are now live! 🎉 (e.g.: change ➡️ site).

@IAmHopp
Copy link
Contributor Author

IAmHopp commented Aug 8, 2018

Amazing! Glad I could help. I'll rebase and wrap this up tomorrow.

Bruno Vinicius Figueiredo dos Santos and others added 18 commits August 9, 2018 10:13
Broken link in hint-highest-available-document-mode.
One of my changes included a fix inside of a quote from an external resource.
Proposed readability improvements in hint-performance-budget.
Fixes for product names specified with lowercase and an unmatched bracket.
Fixes an extraneous comma and improves cohesion inside of a bullet point declaration.
One change to improve conciseness and one to improve readability by removing a "browsers pages" collision.
Proposed change to improve conciseness.
Cohesion improvement in hint-http-compression.
Conforming to "and/or", which is most used elsewhere in the docs.
@IAmHopp
Copy link
Contributor Author

IAmHopp commented Aug 9, 2018

Alright, guys, One last case I want to report: The "Can the hint be configured?" section in hint-meta-viewport looks sort of broken. The information that's there seems to belong elsewhere, which, if true, would leave it devoid of content, of course. I think one of you should take a look.

As for the other files, I think I'm done. Let me know if you need my help elsewhere.

@molant
Copy link
Member

molant commented Aug 9, 2018

Thanks a lot @IAmHopp!

Let me know if you need my help elsewhere.

All the issues that don't have someone assigned are available that wants to do it and we can help in the process 😊
Do you want to do some coding or continue with documentation? It will be good to review the other sections of the user and contributor guides and make sure they make sense and everything is well structured.

Re the meta-viewport rule, maybe we can have something similar to:

This hint can not be configured. It takes into consideration the targeted browsers, and if no versions of Safari for iOS < 9 are included, it will not require initial-scale=1.

@alrra
Copy link
Contributor

alrra commented Aug 9, 2018

The information that's there seems to belong elsewhere

The information belongs there, but yes, it can be improved.

As for the other files, I think I'm done.

I'll start reviewing and merge this PR.

Again, thank you so much for doing this, we sincerely appreciate it!. 💜


@IAmHopp Did you learn anything new going through the docs?

@alrra alrra closed this in 101ba4d Aug 9, 2018
@@ -35,13 +35,13 @@ As for the charset meta tag, always use `<meta charset="utf-8">` as:
example]. The same may be true for other agents (non-browsers) that
may scan/get the content and may not have the alias.

* Must be inside the `<head>` element and [within the first 1024
* It must be inside the `<head>` element and [within the first 1024
Copy link
Contributor

Choose a reason for hiding this comment

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

It needs to

@IAmHopp IAmHopp deleted the patch-1 branch August 10, 2018 00:23
@IAmHopp
Copy link
Contributor Author

IAmHopp commented Aug 10, 2018

It will be good to review the other sections of the user and contributor guides and make sure they make sense and everything is well structured.

Alright, I'll get on it before venturing into the code.

@IAmHopp Did you learn anything new going through the docs?

Sure! I had already read most of the pages before, but it was cool getting to know the whole thing. I'm quite fond of this project.

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