Skip to content

Conversation

@BrianHenryIE
Copy link
Member

@BrianHenryIE BrianHenryIE commented Sep 20, 2023

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.

@BrianHenryIE BrianHenryIE requested a review from a team as a code owner September 20, 2023 19:24
Copy link
Member

@danielbachhuber danielbachhuber left a 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!

@BrianHenryIE
Copy link
Member Author

@danielbachhuber Yes, please. I've further changes planned – in particular to make things faster. Currently, even when /vendor is in the .distignore, it's comparing every file inside vendor against the rules!

@swissspidy
Copy link
Member

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.

Copy link
Member

@danielbachhuber danielbachhuber left a 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 !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants