Skip to content

Change var and let where necessary #51

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Jan 8, 2017
Merged

Change var and let where necessary #51

merged 3 commits into from
Jan 8, 2017

Conversation

schoenwaldnils
Copy link
Contributor

As discussed in #31 here my implementation of const and let.

@schoenwaldnils schoenwaldnils mentioned this pull request Jan 6, 2017
Closed
@vsemozhetbyt
Copy link
Contributor

Unfortunately, now we have at least two concurrent full attempts (#38 and #51) and two partial attempts (#33 and #44) to address the issue.

@schoenwaldnils
Copy link
Contributor Author

schoenwaldnils commented Jan 6, 2017 •
edited
Loading

#33 is more an attempt to show the use of the variable-name, isn't it?
#44 is included in #51

#38 defaults to let.
In my understanding let only should be used where the variable is reassigned.
But constcan be alternated. (#50)
For example the eslint-decleration prefer-const

ryanmcdermott
ryanmcdermott requested changes Jan 7, 2017
View reviewed changes
Copy link
Owner

@ryanmcdermott ryanmcdermott left a comment

Choose a reason for hiding this comment

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

This is looking great! Make these changes and I think this is good to go, unless anyone has objections. Thanks so much for taking the time to make all of this consistent. I appreciate your constant support ;)

README.md Outdated

it('handles leap year', function() {
let date = new MakeMomentJSGreatAgain('2/1/2016');
cosnt date = new MakeMomentJSGreatAgain('2/1/2016');
Copy link
Owner

Choose a reason for hiding this comment

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

Should be const

Copy link
Contributor Author

Choose a reason for hiding this comment

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

my bad :D

README.md Outdated

it('handles non-leap year', function() {
let date = new MakeMomentJSGreatAgain('2/1/2015');
cosnt date = new MakeMomentJSGreatAgain('2/1/2015');
Copy link
Owner

Choose a reason for hiding this comment

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

Should be const

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is fixed now

This was referenced Jan 7, 2017
Closed
Merged
Nils Schönwald added 2 commits January 7, 2017 15:06
Fix typo
2200e40
Merge branch 'master' of github.com:ryanmcdermott/clean-code-javascript
63ca186 
* 'master' of github.com:ryanmcdermott/clean-code-javascript:
  Add: Error Handling Section
  Correct spelling of Cessna airplane
  Adjusted the commentthat show the result
@ryanmcdermott ryanmcdermott merged commit 62f0b70 into ryanmcdermott:master Jan 8, 2017
@ryanmcdermott
Copy link
Owner

This looks great! Since sadly the examples in this document aren't testable (maybe we could find a way in the future), I assume that everything here works after reading it as carefully as I could. If anyone sees anything else that stands out with these changes, open an issue or ideally a PR 😄

This was referenced Jan 8, 2017
Closed
Closed
Merged
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ryanmcdermott ryanmcdermott ryanmcdermott requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@schoenwaldnils @vsemozhetbyt @ryanmcdermott