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

doc: updating REPLACEME tag during release #7514

Closed
wants to merge 1 commit into from

Conversation

gibfahn
Copy link
Member

@gibfahn gibfahn commented Jul 1, 2016

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

doc

Description of change

Add a paragraph to the releases.md guide to document replacing the
REPLACEME tag with the release version for new APIs.

Ref: #6864 (comment), #6578

@addaleax @claudiorodriguez @evanlucas

Sorry, something went wrong.

@nodejs-github-bot nodejs-github-bot added the doc Issues and PRs related to the documentations. label Jul 1, 2016
@gibfahn gibfahn force-pushed the pr-replaceme-doc branch from 01175d8 to b9d4415 Compare July 1, 2016 21:38
@gibfahn
Copy link
Member Author

gibfahn commented Jul 1, 2016

@claudiorodriguez I wasn't sure what to add to CONTRIBUTING.md, it's quite a short document at the moment (which is a good thing IMO), and I guess people adding new APIs are probably already familiar with the contribution process.

@gibfahn gibfahn force-pushed the pr-replaceme-doc branch 2 times, most recently from 49378a2 to f5c2bc8 Compare July 1, 2016 21:45
@evanlucas
Copy link
Contributor

Can we limit to 80 columns? Also, I believe the -i flag requires an argument on OS X. So something like sed -i "" "s/REPLACEME/$VERSION/g" doc/api/*.md

@@ -145,9 +145,13 @@ is shown in **bold** in the index. When updating the index, please make sure
to update the display accordingly by removing the bold styling from the previous
release.

#### Step 3: Update any REPLACEME tags in the docs

If this release includes new APIs then we need to document that they were first added in this version. The relevant commits should already include REPLACEME tags as per the example in the [docs README](../tools/doc/README.md). Check for these tags with `grep REPLACEME doc/api/*.md`, and substitute this node version with `sed -i "s/REPLACEME/$VERSION/g" doc/api/*.md` or `perl -pi -e "s/REPLACEME/$VERSION/g" doc/api/*.md`.
Copy link
Member

Choose a reason for hiding this comment

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

For new additions to the doc, it would be preferred to avoid uses of words like we, you, etc. For instance, the first sentence can be reworded as "If this release includes new APIs, then it is necessary to document that those were first added in this version."

Copy link
Member

Choose a reason for hiding this comment

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

s/REPLACEME/REPLACEME

@gibfahn gibfahn force-pushed the pr-replaceme-doc branch from f5c2bc8 to daac6d0 Compare July 4, 2016 09:25
Add a paragraph to the releases.md guide to document replacing the
REPLACEME tag with the release version for new APIs.
@gibfahn gibfahn force-pushed the pr-replaceme-doc branch from daac6d0 to 3be7c13 Compare July 4, 2016 09:30
@gibfahn
Copy link
Member Author

gibfahn commented Jul 4, 2016

@jasnell Done, thanks.

@evanlucas Should be 80 columns now. As for the sed problem, it seems that the OSX version is incompatible with the standard GNU version. Judging from these (one, two) Stack Overflow answers, it looks like the easiest method is to use the perl command instead (perl should be installed by default), or brew install gnu-sed.

Perl command: perl -pi -e "s/REPLACEME/$VERSION/g" doc/api/*.md

@addaleax
Copy link
Member

addaleax commented Jul 4, 2016

LGTM if the release folks are happy, and I have made the experience that cross-platform sed -i is not really doable, too.

@jasnell
Copy link
Member

jasnell commented Jul 4, 2016

LGTM

@mhdawson
Copy link
Member

mhdawson commented Jul 5, 2016

LTGM

@mhdawson
Copy link
Member

mhdawson commented Jul 6, 2016

@thealphanerd can you review/comment as a "release folk"

@evanlucas
Copy link
Contributor

LGTM

@mhdawson mhdawson self-assigned this Jul 6, 2016
jasnell pushed a commit that referenced this pull request Jul 6, 2016
Add a paragraph to the releases.md guide to document replacing the
REPLACEME tag with the release version for new APIs.

PR-URL: #7514
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
@jasnell
Copy link
Member

jasnell commented Jul 6, 2016

Landed in b10ee9d

@jasnell jasnell closed this Jul 6, 2016
@rvagg
Copy link
Member

rvagg commented Jul 7, 2016

yikes .. you know this is going to get forgotten regularly and will require follow-up PRs to address it. Perhaps it's time to start scripting some of the release stuff, bumping version, ABI #, signed tagging, maybe even have something prompt with a checklist you have to answer to push a release out, it's getting kind of complicated.

@rvagg
Copy link
Member

rvagg commented Jul 7, 2016

@nodejs/release note the new requirement for releases added in here

@addaleax
Copy link
Member

addaleax commented Jul 7, 2016

@rvagg You’ve seen 2cd99eb?

@rvagg
Copy link
Member

rvagg commented Jul 7, 2016

nope, obviously in my backlog that I haven't got to yet! thanks @addaleax, that should help

evanlucas pushed a commit that referenced this pull request Jul 13, 2016
Add a paragraph to the releases.md guide to document replacing the
REPLACEME tag with the release version for new APIs.

PR-URL: #7514
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants