-
Notifications
You must be signed in to change notification settings - Fork 71
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
Pin magika version to fix pip issue #1179
Conversation
63851c6
to
3e619b4
Compare
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 for adding a version
argument to the VM-Install-With-Pip
function @emtuls! 👍
# Conditionally add version string | ||
$toolNameForPip = $toolName | ||
if (![string]::IsNullOrEmpty($version)) { # Check if $version is not empty or null | ||
$toolNameForPip += "==$version" |
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.
I think we should expect the version to include the ==
:
$toolNameForPip += "==$version" | |
$toolNameForPip += "$version" |
This allows us more flexibility in case we need for example to specify a range of versions. I would then also add a comment next to the version
argument to document this:
[string] $version = "", # Version using pip format, example: "==0.5.0"
This also allows us to simplify the code, as the if
is not needed anymore needed? As the version is set to empty by default, it can't be null, only set or empty. Concatenating an empty string makes no different, so we can just do directly:
VM-Pip-Install "$toolName$version"
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.
Tested locally, both installation and uninstallation work as expected! Thanks for all the work @emtuls!!! 😄
This pins
magika
to version0.5.0
to fix a pip resolving issue withnumpy
conflicting withstringsifter
as noted in #1130. It should be fixed whenmagika
version0.6.0
is officially released and we can revert back to how we installed it before after verifying.