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

Remove unnecessary write_delete_marker_file #7653

Merged

Conversation

chrahunt
Copy link
Member

InstallRequirement only checks for a delete marker file in source_dir.
Since the result of ensure_build_location is set to source_dir (in
ensure_has_source_dir), and ensure_build_location returns a subdirectory
of the build_directory to which we write the delete marker file, then
this delete marker file is never used.

Progresses #7571.

@chrahunt chrahunt added type: refactor Refactoring code skip news Does not need a NEWS file entry (eg: trivial changes) labels Jan 25, 2020
InstallRequirement only checks for a delete marker file in source_dir.
Since the result of ensure_build_location is set to source_dir (in
ensure_has_source_dir), and ensure_build_location returns a subdirectory
of the build_directory to which we write the delete marker file, then
this delete marker file is never used.
@chrahunt chrahunt force-pushed the refactor/remove-unnecessary-marker-file branch from c8afd55 to d028af9 Compare January 25, 2020 23:09
@chrahunt chrahunt marked this pull request as ready for review January 26, 2020 00:10
@chrahunt chrahunt mentioned this pull request Jan 28, 2020
12 tasks
@chrahunt
Copy link
Member Author

Thank you for reviewing @sbidoul

I will merge this, but if anyone sees an issue later I'm more than happy to address it in a follow up.

@chrahunt chrahunt merged commit b6ecc39 into pypa:master Jan 29, 2020
@chrahunt chrahunt deleted the refactor/remove-unnecessary-marker-file branch January 29, 2020 00:59
@pradyunsg
Copy link
Member

@chrahunt I think this should be fine but I'm not a 100% sure (and I never actually managed to look into this in a single sitting, because I ended up prioritizing other stuff over this).

@lock lock bot added the auto-locked Outdated issues that have been locked by automation label Mar 10, 2020
@lock lock bot locked as resolved and limited conversation to collaborators Mar 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
auto-locked Outdated issues that have been locked by automation skip news Does not need a NEWS file entry (eg: trivial changes) type: refactor Refactoring code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants