Give more specific errors when rcedit cannot set an icon by malept · Pull Request #74 · electron/rcedit · GitHub
Skip to content
This repository was archived by the owner on Apr 6, 2026. It is now read-only.

Give more specific errors when rcedit cannot set an icon - #74

Merged
zcbenz merged 1 commit into
electron:masterfrom
malept:better-icon-errors
May 7, 2018
Merged

zcbenz merged 1 commit into
electron:masterfrom
malept:better-icon-errors

Conversation

@malept

@malept malept commented May 2, 2018

Copy link
Copy Markdown
Member

In the Electron Packager and Electron Forge issue trackers, one of the common issues that pops up is people asking what's going on when the build process errors out with rcedit.exe failed with exit code 1. Fatal error: Unable to set icon. This pull request attempts to give more details as to why setting the icon failed.

@ckerr ckerr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LOG(ERROR) << ... would be the more idiomatic way to do this in Electron's source.

Other than that, LGTM. Thanks for the patch!

@MarshallOfSound

Copy link
Copy Markdown
Member

@malept

malept commented May 2, 2018

Copy link
Copy Markdown
Member Author

I'd be happy to use a more idiomatic way to deal with errors, but my knowledge of the C++/Windows ecosystem is minimal, so assistance would be much appreciated.

@zcbenz zcbenz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For this tiny project fprintf looks good enough, currently it does not have LOG(ERROR) which is a thing from Chromium.

@zcbenz
zcbenz merged commit fc54d9f into electron:master May 7, 2018
@malept
malept deleted the better-icon-errors branch May 7, 2018 05:49
@malept

malept commented May 7, 2018

Copy link
Copy Markdown
Member Author

Thanks @zcbenz. When can we expect a new release of rcedit and node-rcedit?

@zcbenz

zcbenz commented May 7, 2018

Copy link
Copy Markdown
Contributor

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants