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

Make in place mean in place #15

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

on2valhalla
Copy link

In place extraction should mean that if you extract a file its contents should be in the same directory as that file. This should apply even for sub directories

In place extraction should mean that if you extract a file its contents should be in the same directory as that file. This should apply even for sub directories
Copy link

@spreadred spreadred left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your changes to the variable names shouldn't be part of this PR, IMO.

Copy link
Author

@on2valhalla on2valhalla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed the variable names back to originals, though they should likely be renamed for readability sake

@spreadred
Copy link

@on2valhalla to be clear, I like your variable name suggestion. I just thought it should be a different a PR - one that refactors the entire project, not just that snippet.

@on2valhalla
Copy link
Author

on2valhalla commented Dec 12, 2018 via email

d8ahazard added a commit to d8ahazard/deluge-extractor that referenced this pull request Oct 25, 2019
In place extraction should mean that if you extract a file its contents should be in the same directory as that file. This should apply even for sub directories.

(Copy of chrishuan9#15)
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.

2 participants