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

fix: Fix unlinking existing binaries #12

Merged
merged 1 commit into from
Feb 17, 2022
Merged

fix: Fix unlinking existing binaries #12

merged 1 commit into from
Feb 17, 2022

Conversation

joeyparrish
Copy link
Member

We failed to unlink binaries because we checked for the existence of
"path" instead of the correct variable name "outputPath". This went
unnoticed because "path" is a global referring to the "path" module,
so the code still ran.

Another factor that made this more difficult to notice is that in
other methods, we clobbered the global module "path" with a method
parameter of the same name. This has also been fixed by renaming
method parameters from "path" to something else.

We failed to unlink binaries because we checked for the existence of
"path" instead of the correct variable name "outputPath".  This went
unnoticed because "path" is a global referring to the "path" module,
so the code still ran.

Another factor that made this more difficult to notice is that in
other methods, we clobbered the global module "path" with a method
parameter of the same name.  This has also been fixed by renaming
method parameters from "path" to something else.
@joeyparrish joeyparrish added the type: bug Something isn't working correctly label Feb 17, 2022
@joeyparrish joeyparrish requested a review from theodab February 17, 2022 19:51
@joeyparrish joeyparrish merged commit 8a43765 into main Feb 17, 2022
@joeyparrish joeyparrish deleted the fix-unlink branch February 17, 2022 20:20
@github-actions github-actions bot added the status: archived Archived and locked; will not be updated label Aug 16, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status: archived Archived and locked; will not be updated type: bug Something isn't working correctly
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants