-
Notifications
You must be signed in to change notification settings - Fork 72
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
Update common.vm for all affected packages #987
Conversation
eb68674
to
e2f9cd1
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.
It looks to me like you are removing code in common.vm by accident
e2f9cd1
to
084b4d4
Compare
That would be correct. Too many ctrl-z's 😬 Thanks for catching that! |
c855eeb
to
61fda2b
Compare
303be5f
to
fd4e741
Compare
fd4e741
to
8b4738d
Compare
@@ -397,7 +399,7 @@ function VM-Install-From-Zip { | |||
VM-Install-Shortcut -toolName $toolName -category $category -executablePath $executablePath -consoleApp $consoleApp -arguments $arguments | |||
Install-BinFile -Name $toolName -Path $executablePath | |||
} | |||
return $executablePath | |||
return ,@($toolDir, $executablePath) |
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.
Strange syntax with this comma. Maybe we should add a comment to clarify what this does, but this shouldn't block the PR as the packages are broken.
From #959, there was an issue with packages not requiring the most up to date
common.vm
leading to many packages to fail in thedaily.yml
check.There was also an issue with some older packages not performing
sha256 checks
due to it not being a mandatory requirement when made, which will need to be fully addressed later.This should fix these issues.