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

Update README file - Extend explanations, fix formatting mistakes and expand examples #2938

Draft
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

bstashchuk
Copy link

I am author of few JavaScript courses on Udemy.
While recording new JavaScript course I used AirBnb Style Guide in one of the sections and noticed few ways for improvements.

  • Expanded examples
  • Fixed some formatting mistakes
  • Extended explanations

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@ljharb ljharb marked this pull request as draft March 7, 2024 19:31
Copy link
Author

@bstashchuk bstashchuk left a comment

Choose a reason for hiding this comment

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

Thanks for the quick feedback @ljharb
I hope we will be able to incorporate few of my changes.

README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
@bstashchuk bstashchuk requested a review from ljharb March 8, 2024 10:05
README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@bstashchuk bstashchuk requested a review from ljharb March 9, 2024 10:46
@bstashchuk
Copy link
Author

@ljharb I am still thinking about 19.6

There are actually general ESLint rules regarding method chains formatting but for some reason all examples are about jQuery and DOM manipulation.

No pure JS examples of methods chaining for arrays or some objects manipulation.

May I reduce quantity of the jQuery examples (or make them less confusing due to the long chains of method calls with different indents) and add some additional examples with bad/good formatting?

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@bstashchuk bstashchuk requested a review from ljharb March 11, 2024 21:25
@bstashchuk bstashchuk marked this pull request as ready for review March 18, 2024 13:40
@bstashchuk
Copy link
Author

Hi @ljharb ,
let me know if something else is needed to change in this PR.
It seems all questions were resolved.
Bogdan

@bstashchuk bstashchuk force-pushed the update-readme branch 2 times, most recently from 352c3db to 891cff2 Compare April 12, 2024 10:31
README.md Show resolved Hide resolved
@ljharb ljharb marked this pull request as draft September 30, 2024 00:38
}

foo()
console.log(bar) // 10
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
console.log(bar) // 10
console.log(bar); // 10

bar = 10 // bar will appear in the global scope
}

foo()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
foo()
foo();

@@ -1646,6 +1653,14 @@ Other Style Guides
// bad
superPower = new SuperPower();

// bad
function foo() {
bar = 10 // bar will appear in the global scope
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
bar = 10 // bar will appear in the global scope
bar = 10; // bar will appear in the global scope

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants