Skip to content
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

Documented "skipUpdateIcon" option. #202

Merged
merged 2 commits into from
Mar 27, 2018
Merged

Documented "skipUpdateIcon" option. #202

merged 2 commits into from
Mar 27, 2018

Conversation

bursauxa
Copy link
Contributor

The option skipUpdateIcon was not documented, but seems necessary when using setupIcon to avoid error messages. I could only find it in a comment buried in a closed issue (#81 (comment)).

The option `skipUpdateIcon` was not documented, but seems necessary when using `setupIcon` to avoid error messages. I could only find it in a comment buried in a closed issue (#81 (comment)).
README.md Outdated
@@ -57,6 +57,7 @@ There are several configuration settings supported:
| `signWithParams` | No | Params to pass to signtool. Overrides `certificateFile` and `certificatePassword`. |
| `iconUrl` | No | A URL to an ICO file to use as the application icon (displayed in Control Panel > Programs and Features). Defaults to the Atom icon. |
| `setupIcon` | No | The ICO file to use as the icon for the generated Setup.exe |
| `skipUpdateIcon` | No | When set to true, prevents setupIcon-related error messages "This application could not be started" that may occur during installation. |
Copy link
Member

Choose a reason for hiding this comment

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

This isn't actually what the skipUpdateIcon option does, it's the effect of enabling the option but it doesn't actually do this. A more appropriate thing to put here would be

Disables setting the icon of `Update.exe`, this can solve startup errors if you are building your application on macOS.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm all for having more accurate documentation, and indeed I focused on the indirect effect rather than the direct one.

However, I never ran/built anything on macOS, so I am not sure that your description is entirely accurate either? Because for sure I needed to enable that option.

@MarshallOfSound MarshallOfSound merged commit 7ebee45 into electron:master Mar 27, 2018
@bursauxa bursauxa deleted the patch-1 branch March 27, 2018 11:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants