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.

Sign up for GitHub

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

Jump to bottom

v7.x backport - doc: correct vcbuild options for windows testing #10686

Closed
wants to merge 1 commit into from
Closed

v7.x backport - doc: correct vcbuild options for windows testing #10686

wants to merge 1 commit into from

Conversation

gibfahn
Copy link
Member

@gibfahn gibfahn commented Jan 8, 2017

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.

Reopened version of #10112, I added .\ and changed test nosign->nosign test as per comments in that PR (and also change commit message to be <50 chars).

Checklist
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

doc, win

@gibfahn gibfahn added doc Issues and PRs related to the documentations. windows Issues and PRs related to the Windows platform. labels Jan 8, 2017
@nodejs-github-bot nodejs-github-bot added doc Issues and PRs related to the documentations. dont-land-on-v7.x labels Jan 8, 2017
@gibfahn
Copy link
Member Author

gibfahn commented Jan 8, 2017

@jboarman I reopened your PR, the commit is still attributed to you, let me know if that's okay.

@gibfahn
Copy link
Member Author

gibfahn commented Jan 8, 2017 •
edited
Loading

@MylesBorins this needs to land on v7, v6, and v4 (but not master), should I retarget this PR to v7.x and then add the lts-watch labels for v4 and v6?

EDIT: Did that, let me know if it's wrong...

@gibfahn
Copy link
Member Author

gibfahn commented Jan 8, 2017

@seishun @bzoz @joaocgreis @richardlau

@jboarman @gibfahn
doc: correct vcbuild options for windows testing
3cf4399 
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.
@gibfahn gibfahn changed the base branch from master to v7.x-staging January 8, 2017 15:20
@mscdex mscdex added the build Issues and PRs related to build files or the CI. label Jan 8, 2017
jasnell pushed a commit that referenced this pull request Jan 10, 2017
@jboarman @jasnell
doc: correct vcbuild options for windows testing
ed9b6c1 
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.

PR-URL: #10686
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Bartosz Sosnowski <bartosz@janeasystems.com>
@jasnell
Copy link
Member

jasnell commented Jan 10, 2017

Landed in ed9b6c1

@jasnell jasnell closed this Jan 10, 2017
@gibfahn gibfahn deleted the vcbuild-nosign branch January 10, 2017 21:53
@gibfahn
Copy link
Member Author

gibfahn commented Jan 16, 2017

@jasnell it looks like ed9b6c1 landed in master? This is actually wrong in master, it was superseded in #10156 (but that was semver-major so didn't go into release lines). It needs to go into v7.x, v6.x, and v4.x (see #10686 (comment)). It's my bad, I should have put a big v7.x sign at the top of the PR.

@nodejs/lts what do we do now? Do we revert this in master?

@gibfahn gibfahn restored the vcbuild-nosign branch January 16, 2017 12:14
@gibfahn gibfahn reopened this Jan 16, 2017
@gibfahn gibfahn changed the title doc: correct vcbuild options for windows testing v7.x - doc: correct vcbuild options for windows testing Jan 16, 2017
@gibfahn gibfahn mentioned this pull request Jan 16, 2017
Merged
2 tasks
@mscdex mscdex added the v7.x label Jan 16, 2017
@jasnell
Copy link
Member

jasnell commented Jan 16, 2017

aw dangit... I must have forgotten to switch branches. ok, thanks for spotting this. glad it wasn't something more substantial

italoacasas pushed a commit to italoacasas/node that referenced this pull request Jan 18, 2017
@jboarman @italoacasas
doc: correct vcbuild options for windows testing
2007e28 
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.

PR-URL: nodejs#10686
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Bartosz Sosnowski <bartosz@janeasystems.com>
@gibfahn gibfahn changed the title v7.x - doc: correct vcbuild options for windows testing v7.x backport - doc: correct vcbuild options for windows testing Jan 19, 2017
italoacasas pushed a commit to italoacasas/node that referenced this pull request Jan 23, 2017
@jboarman @italoacasas
doc: correct vcbuild options for windows testing
2e64542 
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.

PR-URL: nodejs#10686
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Bartosz Sosnowski <bartosz@janeasystems.com>
italoacasas pushed a commit to italoacasas/node that referenced this pull request Jan 24, 2017
@jboarman @italoacasas
doc: correct vcbuild options for windows testing
05bab2c 
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.

PR-URL: nodejs#10686
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Bartosz Sosnowski <bartosz@janeasystems.com>
@italoacasas italoacasas mentioned this pull request Jan 25, 2017
Closed
italoacasas pushed a commit to italoacasas/node that referenced this pull request Jan 27, 2017
@jboarman @italoacasas
doc: correct vcbuild options for windows testing
3f17757 
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.

PR-URL: nodejs#10686
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Bartosz Sosnowski <bartosz@janeasystems.com>
@italoacasas italoacasas mentioned this pull request Jan 29, 2017
Merged
@joaocgreis
Copy link
Member

This has landed correctly in v7.x-staging and was correctly reverted in master. But still needs to land in v4.x-staging and v6.x-staging. Since this is approved, I'll just cherry-pick the commit to those branches. I'll do this tomorrow to give @nodejs/lts a chance to stop me if this is not correct or should be done in some other way.

@MylesBorins
Copy link
Contributor

@joaocgreis please hold off on backporting to staging. We generally don't land anything until it has lived in master for at least two weeks. We are also mid release cycle, so anything on staging will need to be rebased. If you want to guarantee that specific commits land together I suggest opening a backport PR

@gibfahn
Copy link
Member Author

gibfahn commented Feb 8, 2017

@MylesBorins this is a docs-only bugfix PR, which also only affects people building from source on Windows (it only changes BUILDING.md and CONTRIBUTING.md). Does it still need to wait for two weeks?

@MylesBorins
Copy link
Contributor

Ah, didn't realize it was docs only... backport away, I can wrap it into the next release

joaocgreis pushed a commit that referenced this pull request Feb 8, 2017
@jboarman @joaocgreis
doc: correct vcbuild options for windows testing
6614bd1 
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.

PR-URL: #10686
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Bartosz Sosnowski <bartosz@janeasystems.com>
@joaocgreis
Copy link
Member

Landed in v6.x-staging in 6614bd1 .

For v4.x-staging, this requires #8704 that doesn't land cleanly but easily fixed. I have the work done locally, just waiting for an ok in that PR to push.

joaocgreis pushed a commit that referenced this pull request Feb 9, 2017
@jboarman @joaocgreis
doc: correct vcbuild options for windows testing
6034bdc 
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.

PR-URL: #10686
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Bartosz Sosnowski <bartosz@janeasystems.com>
@joaocgreis
Copy link
Member

Landed in v4.x-staging in 6034bdc .

I believe we now have this correct in all branches.

@joaocgreis joaocgreis closed this Feb 9, 2017
MylesBorins pushed a commit that referenced this pull request Feb 21, 2017
@jboarman @MylesBorins
doc: correct vcbuild options for windows testing
d8961bd 
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.

PR-URL: #10686
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Bartosz Sosnowski <bartosz@janeasystems.com>
MylesBorins pushed a commit that referenced this pull request Feb 21, 2017
@jboarman @MylesBorins
doc: correct vcbuild options for windows testing
76e6e7e 
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.

PR-URL: #10686
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Bartosz Sosnowski <bartosz@janeasystems.com>
@gibfahn gibfahn deleted the vcbuild-nosign branch June 3, 2017 14:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jasnell jasnell jasnell approved these changes

@bzoz bzoz bzoz approved these changes

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.
8 participants
@gibfahn @jasnell @joaocgreis @MylesBorins @bzoz @mscdex @nodejs-github-bot @jboarman