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

V6.9.1 proposal #9186

Merged
merged 7 commits into from
Oct 19, 2016
Merged

V6.9.1 proposal #9186

merged 7 commits into from
Oct 19, 2016

Conversation

MylesBorins
Copy link
Contributor

2016-10-19, Version 6.9.1 'Boron' (LTS), @thealphanerd

Notable changes

  • streams: Fix a regression in readable stream that caused unpipe to remove the wrong stream (Anna Henningsen)

Commits

  • [2c3bbb576c] - doc: fix changelog index for v6.9.0 (Rod Vagg) #9168
  • [f4b766f5b7] - streams: fix regression in unpipe() (Anna Henningsen) #9171
  • [6072326009] - test: add regression test for unpipe() (Niels Nielsen) #9171
  • [9f248a4d83] - tools: check tag is on github before release (Rod Vagg) #9142
  • [c74d3a700a] - tools: make detached SHASUM .sig file for releases (Rod Vagg) #9071
  • [955bbf876f] - tools: explicitly set digest algo for SHASUM to 256 (Rod Vagg) #9071

rvagg and others added 6 commits October 19, 2016 14:06
PR-URL: #9168
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
PR-URL: #9071
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
PR-URL: #9071
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
PR-URL: #9142
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Since 2e568d9 there is a bug where unpiping a stream
from a readable stream that has `_readableState.pipesCount > 1`
will cause it to remove the first stream in the
`_.readableState.pipes` array no matter where in the list the
`dest` stream was.

This patch corrects that problem.

PR-URL: #9171
Fixes: #9170
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Since 2e568d9 there is a bug where unpiping a stream
from a readable stream that has `_readableState.pipesCount > 1`
will cause it to remove the first stream in the
`_.readableState.pipes` array no matter where in the list the
`dest` stream was.

PR-URL: #9171
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
@MylesBorins MylesBorins added meta Issues and PRs related to the general management of the project. lts Issues and PRs related to Long Term Support releases. v6.x labels Oct 19, 2016
@MylesBorins
Copy link
Contributor Author

The only commits we absolutely need to include in this release are f4b766f and 6072326

I've included the other commits that were on v6.x-staging as they were all tooling related and likely to make the release process smoother.

ci: https://ci.nodejs.org/job/node-test-pull-request/4576/
citgm: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/427/

/cc @nodejs/ctc @nodejs/lts

@@ -39,7 +40,23 @@
[Node.js Long Term Support Plan](https://github.com/nodejs/LTS) and
will be supported actively until April 2018 and maintained until April 2019.

<a id="6.8.1"></a>
<a id="6.9.0"></a>
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be 6.9.1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

MylesBorins pushed a commit to MylesBorins/node that referenced this pull request Oct 19, 2016
Notable changes:

* streams: Fix a regression in readable stream that caused unpipe to
remove the wrong stream (Anna Henningsen)

PR-URL: nodejs#9186
@Fishrock123
Copy link
Contributor

Do we want to do this this week?

It sounds like this was only an issue for a minority of users if it wasn't caught for v6.8.1 or v6.9.0...?

Personally I would probably just do a patch next week unless this is breaking a lot of users.

@evanlucas
Copy link
Contributor

I'm +1 for a release this week. Especially since v6.x fixed a few security issues.

@jasnell
Copy link
Member

jasnell commented Oct 19, 2016

As long as we can get this out today I'm +1. Thursday and Friday releases aren't the best.

@MylesBorins
Copy link
Contributor Author

MylesBorins commented Oct 19, 2016

smart os and arm failures are infra related.

Local OSX is passing, so I think we can ignore those.

Going to rerun BSD one more time: https://ci.nodejs.org/job/node-test-commit-freebsd/4901/

Build: https://ci-release.nodejs.org/job/iojs+release/1273/

edit: citgm failures included a body-parser failure on osx and a core-dump with async on fedora 22.

I have tested locally to confirm that the body-parser failure is not repeatable, currently ssh'd into a fedora box and compiling to confirm that it cannot be reproduced there

edit:

Manually tested async on fedora, no issues. CITGM looks good

@MylesBorins
Copy link
Contributor Author

Still a single failure on BSD
https://ci.nodejs.org/job/node-test-commit-freebsd/4901/nodes=freebsd11-x64/tapTestReport/test.tap-970/

@nodejs/platform-freebsd is this concerning to you?

@jbergstroem
Copy link
Member

@thealphanerd I'm in no shape to have a look at it.

@Fishrock123
Copy link
Contributor

Based on the reports received, not sure this is not high enough priority to warrant another release this week.

@mhdawson
Copy link
Member

Same as James: "As long as we can get this out today I'm +1. Thursday and Friday releases aren't the best."

Notable changes:

* streams: Fix a regression introduced in v6.8.0 in readable stream
that caused unpipe to remove the wrong stream (Anna Henningsen)

PR-URL: #9186
@MylesBorins MylesBorins merged commit 6eebe68 into v6.x Oct 19, 2016
MylesBorins pushed a commit that referenced this pull request Oct 19, 2016
MylesBorins pushed a commit that referenced this pull request Oct 19, 2016
Notable changes:

* streams: Fix a regression introduced in v6.8.0 in readable stream
that caused unpipe to remove the wrong stream (Anna Henningsen)

PR-URL: #9186
@addaleax
Copy link
Member

@Fishrock123 I’m curious – if there is something that’s clearly a regression with an easy fix and there’s a releaser willing to put together a release, what are generally the reasons not to do one?

@MylesBorins MylesBorins deleted the v6.9.1-proposal branch December 15, 2016 18:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lts Issues and PRs related to Long Term Support releases. meta Issues and PRs related to the general management of the project.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants