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

Add documentation for version-string #30

Closed
wants to merge 1 commit into from

Conversation

jesobreira
Copy link

node-rcedit was also suffering the lack of documentation for version-string mentioned on electron/rcedit#2

This PR adds to the README examples of usage of the object, as seen on unindented/grunt-rcedit

By the way, unindented/grunt-rcedit#1 has added a table to the README that lists every possible key and explains what it means. However it doesn't fit to me to tell whether this repo must bring that information or if users should find that information on official sources (i.e.: MSDN).

Copy link
Member

@malept malept left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! I've made some suggestions below.

'LegalCopyright': 'Copyright (C) Acme Inc.',
'ProductName': 'My Test'
}

Copy link
Member

Choose a reason for hiding this comment

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

unnecessary whitespace


```
var options = {

Copy link
Member

Choose a reason for hiding this comment

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

unnecessary whitespace

`version-string` allows you to set the PE file meta keys, such as in the following example:

```
var options = {
Copy link
Member

Choose a reason for hiding this comment

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

This block should probably go in an "options example" section (I'm not entirely sure what the section should be titled though), since it's showing more than just an example of version-string.

@@ -37,6 +37,25 @@ On platforms other than Windows, you will need to have [Wine](http://winehq.org)
* `application-manifest` - String path to a local manifest file to use.
See [here](https://msdn.microsoft.com/en-us/library/windows/desktop/aa374191.aspx)
for more details.

`version-string` allows you to set the PE file meta keys, such as in the following example:
Copy link
Member

Choose a reason for hiding this comment

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

This should have a link to the MSDN docs, like application-manifest does.

@malept malept changed the title Documenting version-string Add documentation for version-string May 9, 2017

`version-string` allows you to set the PE file meta keys, such as in the following example:

```
Copy link
Contributor

Choose a reason for hiding this comment

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

Might want to make this js so it highlights.


```js

@ckerr
Copy link
Member

ckerr commented Aug 10, 2018

Closing stale PRs. This PR has not addressed the last code review from over a year ago.

Please reopen or resubmit this PR after addressing the reviewer's comments. Thank you!

@ckerr ckerr closed this Aug 10, 2018
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.

4 participants