-
Notifications
You must be signed in to change notification settings - Fork 6.3k
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
Rename 'Stable' release to 'Current' #672
Conversation
@fhemberger thanks for getting this started. I'll take a proper look tomorrow |
I think this is unrelated to this change, I sporadically received the same error in the last days. |
other-stable-downloads: Altri Download Stabili | ||
stable: Stabile | ||
other-current-downloads: Altri Download Stabili | ||
current: Stabile |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
current: Corrente
8222f49
to
89a783c
Compare
other-stable-downloads: Altri Download Stabili | ||
stable: Stabile | ||
other-current-downloads: Altri Download Stabili | ||
current: Corrente |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lpinca Grazie! ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is the exact kind of stuff I knew I would miss. Thanks for getting on it so quickly @fhemberger !!!
@fhemberger @lpinca I think https://github.com/nodejs/nodejs.org/blob/master/server.js#L44 |
const github = nock('https://raw.githubusercontent.com') | ||
.get('/nodejs/node/v4.1.0/CHANGELOG.md') | ||
.replyWithFile(200, changelogFixture) | ||
|
||
releasePost.fetchVersionPolicy('4.1.0').then((policy) => { | ||
t.equal(policy, 'Stable') | ||
t.equal(policy, 'Current') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This assertion fails because the channel is resolved as "Stable" from the fixture.
Maybe it makes sense to create a new fixture faking a changelog for the "Current" channel.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the name change affect existing releases? If not, this assertion should not be changed as v4.1.0 should still be considered "stable".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the name change affect existing releases?
I vote yes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I vote yes.
I would agree in the eyes of consistency, but that would possibly mean updating all previous changelogs/blogs etc, I'm not sure that's preferable.
@fhemberger as I'm not sure what to do about previous releases, I've made two branches with contains fixes for both scenarios:
rename-stable-old-still-stable
: old releases should still be named "Stable"rename-stable-incl-old-releases
: rename old releases to "Stable" as well
Merge or cherry-pick what ever commit you see fit. IMO keeping "Stable" for old releases would be okey for this PR, further discussion and a permanent fix could be raised in a separate issue/PR.
@abouthiroppy it makes sense. |
@abouthiroppy Good catch, thanks! Crash is fixed. |
@node/ctc ... FYI |
@@ -6,10 +6,10 @@ labels: | |||
download-for: Download for | |||
other-downloads: 다른 운영 체제 | |||
other-lts-downloads: 다른 LTS 다운로드 | |||
other-stable-downloads: 다른 안정 버전 다운로드 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fhemberger Could you change '안정' to '현재' on here? ('stable' to 'current' in Korean)
Any other things are LGTM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@preco21 Updated the translation, also on the next line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fhemberger Cool! Thank you.
@phillipj I went with the old releases to be changed as well. Updated release posts and weekly updates for v4.x/5.x releases. Anything else missing or are we good? |
@@ -2,7 +2,7 @@ | |||
date: 2015-09-23T02:01:38.545Z | |||
version: 4.1.1 | |||
category: release | |||
title: Node v4.1.1 (Stable) | |||
title: Node v4.1.1 (Current) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not under the impression that we will be changing the release type of old releases... these posts should likely remain unchanged
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just seeing that this was discussed above. I for one would push towards not making a decision in this PR and supporting the old names. There is no way for us to update release commits, and I do not believe there is any intention of modifying the changelogs. It would be odd if the release posts + changelogs are not the same
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's why I did this in a separate commit, so the others can be cherry-picked, if needed.
I'm going to merge this. We have other messaging lined up around this terminology going out today and the website should line up with it. So far there have been zero objections from any member of any WG. |
@mikeal We need this redirect to land as well, otherwise the old download links for stable builds will be broken: nodejs/build#399 |
also: should we note this as news and post about it? just to archive it officially |
I will be including a note about the change in the v6 release notes. |
so this was landed with change to all of the past blog posts from stable -> current. We might want to revisit this change as it is going to create a delta from the changelog and releases where this is not going to change |
/cc @jasnell |
With the change of release names from 'Stable' to 'Current' (introduced with Node.js v6), the old download page needs to be redirected. PR-URL: #399 Refs: nodejs/node#6318 Refs: nodejs/nodejs.org#672 Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
PR for #669:
/<lang>/downloads/stable
to/<lang>/downloads/current
: www: redirect from '/<lang>/downloads/stable' to '/<lang>/downloads/current' build#399Changing markdown files while runningFixed (cherry-picked tonpm run serve
crashes the process:master
)Some tests are failing for release blogpost generation, @phillipj can you please take a look?