Skip to content

Conversation

@chroberino
Copy link

No description provided.

robertsLando
robertsLando previously approved these changes Sep 17, 2025
@robertsLando robertsLando changed the title Add new doc section about injecting Windows exe metadata docs: instructions for injecting Windows exe metadata Sep 17, 2025
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds comprehensive documentation for injecting Windows executable metadata into pkg-generated binaries. The documentation addresses the common issue where packaged executables inherit Node.js binary metadata instead of application-specific information.

Key changes:

  • Adds a new section explaining why Windows exe metadata customization is important
  • Provides detailed instructions for using resedit to post-process executables
  • Includes both Node.js API and command-line usage examples

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

```

As an alternative to the Node.js API,
[`resedit`](https://www.npmjs.com/package/resedit) also supports command–line
Copy link

Copilot AI Sep 17, 2025

Choose a reason for hiding this comment

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

[nitpick] Use an em dash (—) instead of an en dash with spaces (– ) for better typographical consistency.

Suggested change
[`resedit`](https://www.npmjs.com/package/resedit) also supports commandline
[`resedit`](https://www.npmjs.com/package/resedit) also supports command-line

Copilot uses AI. Check for mistakes.
README.md Outdated
Comment on lines 595 to 600
npx resedit dist/bin/app.exe dist/bin/app_with_metadata.exe `
--icon 1,dist/favicon.ico `
--company-name "ACME Corporation" `
--file-description "ACME CLI Tool" `
--product-name "ACME Application" `
--file-version 1.2.3.4
Copy link

Copilot AI Sep 17, 2025

Choose a reason for hiding this comment

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

The PowerShell example shows line continuation with backticks, but lacks explanation that this is PowerShell-specific syntax. Consider adding a note that this is PowerShell syntax or provide cross-platform alternatives for bash/cmd users.

Copilot uses AI. Check for mistakes.
Copy link
Member

Choose a reason for hiding this comment

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

Considering that there might be users that cross compile this maybe it's worth adding also a linux version of this

Copy link
Author

Choose a reason for hiding this comment

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

@robertsLando I am not having much experience with Linux binaries, not sure if the same metadata principles will apply there. And especially resedit (which the documented approach is based on) is designed for Windows binaries only:

resedit-js is a library that manipulates resouces contained by Windows Executable files.
(https://www.npmjs.com/package/resedit)

Copy link
Member

Choose a reason for hiding this comment

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

@chroberino you may have mis-understood my comment, I simply said that someone may want to cross compile windows binaries on linux and still be able to customize the exe under Linux, so I proposed to create a linux version of rsedit command that actually you did with powershell syntax :)

Copy link
Author

Choose a reason for hiding this comment

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

Ohhh, got it.
Well, I see two options to address this:

  1. Provide two variants of this comand line example which differ only in the line continuation characters (` vs. \)
  2. Remove the line continuation characters and make the comand line example a one-liner

Which one do you prefer?

Copy link
Member

Choose a reason for hiding this comment

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

I would go for option 1 🙏🏼

@chroberino chroberino marked this pull request as ready for review September 18, 2025 07:30
Copy link
Member

@robertsLando robertsLando left a comment

Choose a reason for hiding this comment

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

LGTM

@robertsLando robertsLando merged commit 14e7df0 into yao-pkg:main Sep 18, 2025
24 checks passed
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.

2 participants