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.

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

Fix JavaScript Lint errors #3654

Merged
merged 4 commits into from
Sep 11, 2019
Merged

Fix JavaScript Lint errors #3654

merged 4 commits into from
Sep 11, 2019

Conversation

javierm
Copy link
Member

@javierm javierm commented Jul 2, 2019

References

Objectives

  • Fix bugs due to typos in variable names
  • Add a test to prevent a potential problems with a JavaScript switch statement
  • Fix an ESLint warning about directly using hasOwnProperty

@javierm javierm self-assigned this Jul 2, 2019
This was referenced Jul 2, 2019
Merged
Closed
Merged
@javierm javierm added this to Testing in Roadmap Sep 10, 2019
@javierm javierm moved this from Testing to Doing in Roadmap Sep 10, 2019
@javierm javierm force-pushed the remove_unused_javascript branch 2 times, most recently from 0ab8fc7 to d8efa18 Compare September 10, 2019 23:44
@javierm javierm moved this from Doing to Testing in Roadmap Sep 10, 2019
Alceu Medeiros and others added 3 commits September 11, 2019 02:05
@javierm
Set current locale as default for datepicker translation
1f059d4
@taitus @javierm
Fix wrong js
10a2e91
@javierm
Add a break to avoid case fallthrough
5ad41d9 
In JavaScript, when there isn't a `break` or `return` statement inside a
`switch` case, the next case will be executed as well.

That wasn't a problem here because CoffeeScript automatically inserts a
`return` statement in this specific situation. However, since we don't
want to return the result of the `hide()` operation, it might be easy to
accidentally remove the `return` statement, causing the code to break.

I've added a test for the scenario where neither `break` nor `return`
statements are present, so we don't run into this error.
@javierm
Don't call hasOwnProperty directly
eab35dc 
Calling it directly might make it difficult to detect bugs, particularly
when we don't trust the data we receive:

https://eslint.org/docs/rules/no-prototype-builtins

If we trust the data we receive, then IMHO we shouldn't even check for
`hasOwnProperty`, since we know what we're going to receive.
@javierm javierm changed the base branch from remove_unused_javascript to master September 11, 2019 00:41
@javierm javierm merged commit de50317 into master Sep 11, 2019
Roadmap automation moved this from Testing to Release 1.1.0 Sep 11, 2019
@javierm javierm deleted the fix_javascript_typos branch September 11, 2019 01:14
smarques pushed a commit to venetochevogliamo/consul that referenced this pull request Apr 29, 2020
@javierm
Merge pull request consuldemocracy#3654 from consul/fix_javascript_typos
956b32e 
Fix JavaScript Lint errors
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@javierm javierm

Labels
Bug
Projects
No open projects
Roadmap
  
Release 1.1.0
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@javierm @taitus