-
Notifications
You must be signed in to change notification settings - Fork 169
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
Review articles from 1st to 21st #104
Open
borntofrappe
wants to merge
17
commits into
wesbos:master
Choose a base branch
from
borntofrappe:master
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* possibly improve the wording behind the `name` variable Re-write the two sentences expressing how including a global variable `name` might cause issues. * possibly improve the wording of the final sentence Include a sentence which possibly closes the article more clearly. * change emphasis of the sentence when highlighting the entire statement the wording is a tad confusing * improve wording behind the quotes pending issue: the link toward [Kyle Simpson's opinion](http://blog.getify.com/constantly-confusing-const/) doesn't actually forward a blog post. The closes match I could find with a quick search is [this URL](https://frontendmasters.com/courses/es6-right-parts/let-vs-var/), but it describes a video on the subject, and not a blog post. * improve working try to simplify a few sentences possibly overly-complicated * try to improve wording break complex sentences in simpler lines * improve wording, add backticks around `this` Backticks added (hopefully) only when needed * Update 02 - let VS const.md grammar Improves grammar and fixes use of the word 'scope' when describing the difference between `let` and `const` * Fix typo in article 14 from pEIces to pIEces * Update 12 - Template Strings Introduction.md Updated Grammar based on Grammarly check
update fork up to master
I found a few sentences rather difficult to understand, and tried to improve on the wording in the most sensible manner. I hope I haven't modified too much the text.
I added two JS snippets to possibly explain the text in a more direct manner.
It seems the link was wrapped in two sets of brackets ```diff - [Ben Allman back in 2010]((http://benalman.com/news/2010/11/immediately-invoked-function-expression/)) + [Ben Alman back in 2010](http://benalman.com/news/2010/11/immediately-invoked-function-expression/) ``` Alman is also written with one l only, but I had to look it up first.
Added the snippet explaining how using `let` instead of `var` results in the aforementioned error message.
these are very nitpicky modifications
These are minor modifications. The codepen embedded at the top might be better described through a link tag, as in `We will be creating [this](http://codepen.io/wesbos/pen/KgpNjJ/)`, but it's more of a design choice. The embed will certainly be better when the markdown file is used in the actual blog.
I've also added the values logged by the console in the respective snippet. I think I saw one or two articles with the same pattern and tried to be consistent.
The last heading was slightly inconsistent compare to the previous ones.
I changed the name included in the second person object to match the name you've used elsewhere in the series.
The last snippet had the `.join()` function applied on the value returned from the `renderKeywords` function, but this one returns a string. On top of this, the `renderKeywords` creates an array of list items, but doesn't remove the commas in between (it is likely the `.join()` function was misplaced).
The code snippets used the value of `strings` and `...values` without specifying the parameters of the function. ```diff - function highlight() { + function highlight(strings, ...values) { ``` I've also tried to improve the wording of a few sentences, but in a very minor way.
I've included a couple of code snippets to refer to the `startsWith()`, `endsWith()` and `includes()` examples. I've also added a `const` for the `leftPad` function. ```diff - leftPad = function(str, length = 20){ + const leftPad = function(str, length = 20){ ```
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Good morning,
I've gone through the first twenty articles and updated the markdown files trying to improve the existing documentation. I tried to avoid re-writing too much, while focusing more on fixing code snippets and a broken link.
There are a lot of commits (one for each markdown file), so if you find them useful it might be best to squash and merge then into a single commit.
Let me know if I overstepped my bounds adding so many modifications.