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

Infer win32metadata from package.json/top-level options #667

Merged
merged 3 commits into from
Jun 11, 2017

Conversation

malept
Copy link
Member

@malept malept commented Jun 6, 2017

  • I have read the contribution documentation for this project.
  • I agree to follow the code of conduct that this project follows, as appropriate.
  • The changes are appropriately documented (if applicable).
  • The changes have sufficient test coverage (if applicable).
  • The testsuite passes successfully on my local machine (if applicable).

Summarize your changes:

Closes #297.
Fixes #611.

@malept malept requested a review from MarshallOfSound June 6, 2017 06:41
MarshallOfSound
MarshallOfSound previously approved these changes Jun 6, 2017
@MarshallOfSound
Copy link
Member

Was reading through this before you even summoned my review 👍 LGTM 🚢

@MarshallOfSound
Copy link
Member

Actually hold up a second, just checking something

@MarshallOfSound MarshallOfSound dismissed their stale review June 6, 2017 06:45

checking something

@MarshallOfSound
Copy link
Member

MarshallOfSound commented Jun 6, 2017

After a quick check of 30 of my installed applications on my Windows machine 27 of them set FileDescription to be the same as ProductName and the other 3 are close just missing some symbols / spacing.

Perhaps it's worth reconsidering that default, not sure why but everyone else seems to be doing it 😕

(Even I'm doing it in GPMDP)

@develar
Copy link
Contributor

develar commented Jun 6, 2017

Perhaps it's worth reconsidering that default, not sure why but everyone else seems to be doing it

Because on bloody windows FileDescription is used as app name in the process view. And if you will use description instead of name — you will get user reports. Details — electron-userland/electron-builder#965

@MarshallOfSound
Copy link
Member

Haha, thanks @develar

That actually reminds me when I had that problem before... That's windows for ya... @malept should probably change it to be productName || name then 😄

@develar
Copy link
Contributor

develar commented Jun 6, 2017

Be aware that you must normalise version string according to *** MS Windows requirements, otherwise Squirrel.Windows will be not happy and it can lead to crash — electron-userland/electron-builder#1101 (comment)

@malept
Copy link
Member Author

malept commented Jun 9, 2017

If there aren't any more concerns about this PR, I'll merge it.

@malept malept merged commit 7fcf8b7 into master Jun 11, 2017
@malept malept deleted the win32metadata-defaults branch June 11, 2017 19: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.

3 participants