-
Notifications
You must be signed in to change notification settings - Fork 23
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
Refactor into get_file_paths_and_names()
function
#81
Refactor into get_file_paths_and_names()
function
#81
Conversation
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.
Let us know when you're ready for a review on this!
@danielbachhuber Yes, please. I've further changes planned – in particular to make things faster. Currently, even when |
Functionality-wise it looks good to me so far 👍 Just merged the latest changes so that tests pass again, which is a good sign too. |
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 again for your work on this, @BrianHenryIE !
Adds a function/moves the logic for determining the source directory, the output directory, the output filename, and the plugin directory name the archive will extract to.
Which required moving the logic to determine the plugin version into its own function too (less attention paid here).
No change in behavior indicated through the tests. This will hopefully address #76 but I do not have a Windows setup to test it on. See the added regex on line 205 which now accounts for Windows absolute paths.
Also an opportunity for some code review as suggested yesterday.