Fix ShellCheck SC2235 issue

[PeterDaveHello]

• https://github.com/koalaman/shellcheck/wiki/SC2235

[ljharb]

I've already got fixes prepared for all the new shellcheck issues.

[PeterDaveHello]

I wonder if this PR should just be closed as the commit was over write but actually shouldn't look like this?

[ljharb]

I intentionally overwrote it; the new commit works just fine.

[PeterDaveHello]

I don't know. If I made some changes, maybe they are just small, does that mean I should be prepared my pull request would be directly overwritten? I don't think GitHub's "Allow edits from maintainers." was designed for this purpose. The changes are the same, so I don't know the reason why that my small work should be taken, even use the same pull request, even worse then directly commit in master branch, but both not good.

[PeterDaveHello]

I believe this is the second time we have this issue, I really appreciate this useful project and your effort, but I don't appreciate this style, don't know any other famous free and open source project maintainer doing the same ...

[ljharb]

Since i already had the commit, the typical approach would be to push it to master and just close the PR. however, i prefer not to litter my repos with orphaned PR refs, so i updated this PR to point to that same commit.

[PeterDaveHello]

You can still close this PR in the same commit, which may be better.

[ljharb]

That would leave an orphaned PR ref.

I don’t understand why a force push of literally anything to a branch on your fork - a branch you’re going to delete the instant the PR is merged or closed - is problematic?

[PeterDaveHello]

An orphaned PR ref doesn't look like a problem to me. I don't see a strong/good reason to overwrite this PR.

[ljharb]

It’s not a problem by default, since by default you’re not fetching PR refs. However, i do fetch them all, so an orphaned ref ends up cluttering my git log.

[ljharb]

Again tho, can you answer the question above? If the alternative is closing the PR and abandoning it (at which point you’ll just delete your branch), and i still use my commit regardless, why do you care if i force push to the PR or not?

[PeterDaveHello]

My commit is good enough in this case and I didn't see a good reason to erase it, I haven't see any other projects doing so.

[ljharb]

Ok, so your objection is unrelated to force pushing, and just that i didn’t use your commit.

Plenty of projects prefer one commit over alternatives - that someone made a PR doesn’t entitle their PR to be merged. It happens tons of times on many projects.

Your commit was fine, but i already had one prepared, and i preferred to use mine. I’m sorry if that upset you, but if that’s going to upset you, then what would be appropriate is to ask maintainers before making a PR in the first place, precisely to avoid feeling upset that your PR might not be directly accepted. eslint, for example, refuses all PRs that don’t first have issues with explicit approval.