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: add shadowed variable loc to message in no-shadow (fixes #13646) #13841
Conversation
Hi @t-mangoe!, thanks for the Pull Request The first commit message isn't properly formatted. We ask that you update the message to match this format, as we use it to generate changelogs and automate releases.
Read more about contributing to ESLint here |
e39cfb5
to
f235f99
Compare
I see. I am waiting. |
#13646 is accepted, so I've labeled this as accepted as well. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@t-mangoe thanks for putting this PR together already! Now that the issue is accepted, let's get it finished up so we can merge it.
I left one idea to improve the message when we don't have location information.
Our contributor license agreement currently thinks you haven't signed it yet. Can you fill it out? If you already have, perhaps it'll catch up when you push another commit to the branch, and if not, let us know and we'll see what's going on.
Hi @t-mangoe!, thanks for the Pull Request The pull request title isn't properly formatted. We ask that you update the message to match this format, as we use it to generate changelogs and automate releases.
Read more about contributing to ESLint here |
Hi @t-mangoe!, thanks for the Pull Request The pull request title isn't properly formatted. We ask that you update the message to match this format, as we use it to generate changelogs and automate releases.
Read more about contributing to ESLint here |
It looks like your latest push included a bunch of unrelated commits from our |
Merge commit t-mangoe@fdd6444 has two parents that look identical to each other. @t-mangoe Did you rebase onto the latest master and then pulled from your branch? If so, You can write $ git status
On branch where-original-identifier
Your branch is up to date with 'origin/where-original-identifier'.
nothing to commit, working tree clean
$ git log -4
commit 6c676213e08277d89af8a85d3445ce5962df57f5 (HEAD -> where-original-identifier, origin/where-original-identifier)
Author: t-mangoe <m.takashi.xpair@gmail.com>
Date: Sat Jan 16 11:08:51 2021 +0900
Update: show message when the variable is global (refs #13646)
commit fdd6444ace10115a74d5d461b1f1a2adb8503323
Merge: 2347d4f93 f235f99e2
Author: t-mangoe <m.takashi.xpair@gmail.com>
Date: Sat Jan 16 11:08:26 2021 +0900
Merge branch 'where-original-identifier' of https://github.com/t-mangoe/eslint into where-original-identifier
commit 2347d4f93ed6f1994cf5ac3cb5a8d1c41add9e50
Author: t-mangoe <m.takashi.xpair@gmail.com>
Date: Sat Nov 14 15:00:49 2020 +0900
Update: show the original identifier place (refs #13646)
commit 6509705a3b8d2542d09d1c22041fe73dd0d0638f
Author: ESLint Jenkins <eslint[bot]@users.noreply.github.com>
Date: Fri Jan 15 18:00:03 2021 -0500
7.18.0
$ git rebase --onto HEAD~2 HEAD~1
First, rewinding head to replay your work on top of it...
Applying: Update: show message when the variable is global (refs #13646)
$ git log -3
commit ...some new hash... (HEAD -> where-original-identifier)
Author: t-mangoe <m.takashi.xpair@gmail.com>
Date: Sat Jan 16 11:08:51 2021 +0900
Update: show message when the variable is global (refs #13646)
commit 2347d4f93ed6f1994cf5ac3cb5a8d1c41add9e50
Author: t-mangoe <m.takashi.xpair@gmail.com>
Date: Sat Nov 14 15:00:49 2020 +0900
Update: show the original identifier place (refs #13646)
commit 6509705a3b8d2542d09d1c22041fe73dd0d0638f
Author: ESLint Jenkins <eslint[bot]@users.noreply.github.com>
Date: Fri Jan 15 18:00:03 2021 -0500
7.18.0
$ git push origin where-original-identifier -f
|
6c67621
to
5da852c
Compare
@btmills @mdjermanovic |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for cleaning that up, much easier to review now. I'm really liking the specific messages for globals vs declared names. Two tiny tweaks then this LGTM. Nice work!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
…646)
Prerequisites checklist
What is the purpose of this pull request? (put an "X" next to an item)
[ ] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[X] Changes an existing rule (template)
[ ] Add autofixing to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:
#13646
What changes did you make? (Give an overview)
I changed the no-shadow rule to show where where the original identifier has been defined.
Is there anything you'd like reviewers to focus on?
I am not familiar with JavaScript, so please check whether my codes have a problem.