Skip to content

Conversation

@TylerLeonhardt
Copy link
Member

@TylerLeonhardt TylerLeonhardt commented Jun 30, 2020

PR Summary

  • Bump rcedit version used
  • Check if ICO file exists before running rcedit

PR Context

rcedit is failing to set icons in CI scenarios because the ICO doesn't exist in the package.

PR Checklist

@iSazonov
Copy link
Collaborator

@TylerLeonhardt Does new version work? Can we merge?

@iSazonov iSazonov added the CL-BuildPackaging Indicates that a PR should be marked as a build or packaging change in the Change Log label Jun 30, 2020
@TylerLeonhardt TylerLeonhardt changed the title Bump rcedit version Handle when the ICO file isn't present Jul 1, 2020
@TylerLeonhardt
Copy link
Member Author

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.

cc @adityapatwardhan

@adityapatwardhan
Copy link
Member

Setting of the icon should be done in packaging instead

@adityapatwardhan
Copy link
Member

Changes should be done here:

$ProductSemanticVersion = Get-PackageSemanticVersion -Version $PackageVersion

@TylerLeonhardt
Copy link
Member Author

@adityapatwardhan do we know if we're using the Daily at that point?

@TravisEz13
Copy link
Member

TravisEz13 commented Jul 2, 2020

If we take this type of fix, I think we should do something more temporary like: https://github.com/PowerShell/PowerShell/pull/13078/files

GitHub
PR Summary Fixes #13077. We now only try to set the icon for pwsh.exe when the ico file is in the payload. PR Context

PR Checklist

PR has a meaningful title

Use the present tense and imperative...

}
(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"
Copy link
Member

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?

@ghost ghost added the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Jul 2, 2020
@TylerLeonhardt
Copy link
Member Author

Closing this in favor of #13123

@ghost ghost removed the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Jul 7, 2020
@TylerLeonhardt TylerLeonhardt deleted the patch-1 branch July 7, 2020 18:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CL-BuildPackaging Indicates that a PR should be marked as a build or packaging change in the Change Log

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants