FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

doc: corrected vcbuild parameters for testing on windows by jboarman · Pull Request #10112 · nodejs/node · GitHub

/ node Public

doc: corrected vcbuild parameters for testing on windows - #10112

Closed
jboarman wants to merge 1 commit into
nodejs:masterfrom
jboarman:my-branch
ConversationCommits (1)ChecksFiles changed
Closed

doc: corrected vcbuild parameters for testing on windows#10112
jboarman wants to merge 1 commit into
nodejs:masterfrom
jboarman:my-branch

Conversation

jboarman commented Dec 4, 2016 •
edited
Loading

Copy link
Copy Markdown
Contributor
Checklist
  • make -j8 test (UNIX), or vcbuild test nosign (Windows) passes
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

doc

Description of change

Corrected parameter for running tests on Windows. Without the corrected
parameters, Windows users encounter an error about failing to sign the
build, "Failed to sign exe", which can be discouraging to new Windows
community members.

doc: corrected vcbuild parameters for testing on windows
990f0ec 
Corrected parameter for running tests on Windows. Without the corrected
parameters, Windows users encounter an error about failing to sign the
build, "Failed to sign exe", which can be discouraging to new Windows
community members.
nodejs-github-bot added the doc Issues and PRs related to the documentations. label Dec 4, 2016
addaleax added build Issues and PRs related to build files or the CI. windows Issues and PRs related to the Windows platform. labels Dec 4, 2016

Trott commented Dec 5, 2016

Copy link
Copy Markdown
Member

@nodejs/platform-windows

seishun approved these changes Dec 5, 2016
View reviewed changes

seishun left a comment •
edited
Loading

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM, although I'd prefer vcbuild nosign test.

gibfahn commented Dec 9, 2016 •
edited
Loading

Copy link
Copy Markdown
Member

I think this would be made unnecessary by #10156, which will also stop people having to remember the nosign argument.

jasnell commented Dec 23, 2016

Copy link
Copy Markdown
Member

#10156 landed. Is this still necessary?

gibfahn commented Dec 23, 2016 •
edited
Loading

Copy link
Copy Markdown
Member

@jasnell #10156 was semver-major, so I guess it still makes sense to do this for v[7,6,4]?

+1 for vcbuild test nosign->vcbuild nosign test

I'd also prefer changing .\vcbuild -> vcbuild for consistency (also on the build line above).

EDIT: we should be using .\vcbuild as per @richardlau's comment

cc/ @joaocgreis

richardlau commented Jan 3, 2017

Copy link
Copy Markdown
Member

@gibfahn .\vcbuild works on both cmd.exe and PowerShell on Windows (vcbuild does not work on PowerShell, see #8704).

joaocgreis approved these changes Jan 5, 2017
View reviewed changes

joaocgreis 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM for v[4,6,7].

jasnell commented Jan 6, 2017

Copy link
Copy Markdown
Member

If this is a backport for 7, 6 and 4, then, separate PRs for landing in those should be opened.

jboarman commented Jan 6, 2017

Copy link
Copy Markdown
Contributor Author

I'm thinking that #10156 now makes this PR un-necessary.

jboarman closed this Jan 6, 2017
jboarman deleted the my-branch branch January 6, 2017 18:44

gibfahn commented Jan 6, 2017 •
edited
Loading

Copy link
Copy Markdown
Member

@jboarman see #10112 (comment), it's necessary for all current release lines as that PR is semver-major.

gibfahn mentioned this pull request Jan 8, 2017
Closed
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

3 more reviewers
joaocgreis joaocgreis approved these changes
seishun seishun approved these changes
bzoz bzoz approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

build Issues and PRs related to build files or the CI. doc Issues and PRs related to the documentations. windows Issues and PRs related to the Windows platform.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

10 participants

Back | FazBrowse Home | New Git URL