-
Notifications
You must be signed in to change notification settings - Fork 8.1k
Handle when the ICO file isn't present #13060
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
Conversation
|
@TylerLeonhardt Does new version work? Can we merge? |
|
This needed more work because apparently the actual issue is that the ICO doesn't exist. I'm tracking down why, but this change is nice because setting the ICO isn't too important. |
|
Setting of the icon should be done in packaging instead |
|
Changes should be done here: PowerShell/tools/packaging/packaging.psm1 Line 1618 in d1aa11e
|
|
@adityapatwardhan do we know if we're using the Daily at that point? |
|
If we take this type of fix, I think we should do something more temporary like: https://github.com/PowerShell/PowerShell/pull/13078/files
|
| } | ||
| (Test-Path "~/.rcedit/rcedit-x64.exe") { | ||
| Write-Verbose "Install RCEdit for modifying exe resources" -Verbose | ||
| $rceditUrl = "https://github.com/electron/rcedit/releases/download/v1.1.1/rcedit-x64.exe" |
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.
Does this need to run on the client machine?
|
Closing this in favor of #13123 |
PR Summary
PR Context
rcedit is failing to set icons in CI scenarios because the ICO doesn't exist in the package.
PR Checklist
.h,.cpp,.cs,.ps1and.psm1files have the correct copyright headerWIP:or[ WIP ]to the beginning of the title (theWIPbot will keep its status check atPendingwhile the prefix is present) and remove the prefix when the PR is ready.