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

resolve lint shadowed error #123

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

bekuno
Copy link
Member

@bekuno bekuno commented Dec 2, 2020

Description:

Variables should not be shadowed
javascript:S1117

Overriding or shadowing a variable declared in an outer scope can strongly impact the readability, and therefore the maintainability, of a piece of code. Further, it could lead maintainers to introduce bugs because they think they're using one variable but are really using another.

Description:

Variables should not be shadowed
javascript:S1117

Overriding or shadowing a variable declared in an outer scope can strongly impact the readability, and therefore the maintainability, of a piece of code. Further, it could lead maintainers to introduce bugs because they think they're using one variable but are really using another.
@bekuno bekuno self-assigned this Dec 2, 2020
@bekuno
Copy link
Member Author

bekuno commented Dec 2, 2020

@capoaira
Please review. I added stupid a number at the end of the problematic variables.

@capoaira
Copy link
Contributor

capoaira commented Dec 3, 2020

LGTM. But maybe it makes more sense if we use let instead of var in all if statements

@bekuno
Copy link
Member Author

bekuno commented Dec 3, 2020

You are right, let makes sense on many places in the code.
I believe such a rework it is better after PR #116.

@Lineflyer
Copy link
Member

@bekuno #116 has been merged. If you want to rebase and add missing changes, feel free to go ahead.

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