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

doc: updating REPLACEME tag during release by gibfahn · Pull Request #7514 · nodejs/node · GitHub

/ node Public

doc: updating REPLACEME tag during release - #7514

Closed
gibfahn wants to merge 1 commit into
nodejs:masterfrom
gibfahn:pr-replaceme-doc
ConversationCommits (1)ChecksFiles changed
Closed

doc: updating REPLACEME tag during release#7514
gibfahn wants to merge 1 commit into
nodejs:masterfrom
gibfahn:pr-replaceme-doc

Conversation

gibfahn commented Jul 1, 2016

Copy link
Copy Markdown
Member
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

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

gibfahn commented Jul 1, 2016

Copy link
Copy Markdown
Member Author

@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 force-pushed the pr-replaceme-doc branch 2 times, most recently from 49378a2 to f5c2bc8 Compare July 1, 2016 21:45

evanlucas commented Jul 3, 2016

Copy link
Copy Markdown
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

jasnell reviewed Jul 3, 2016
View reviewed changes
Comment thread doc/releases.md Outdated

jasnell Jul 3, 2016

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

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

jasnell Jul 3, 2016

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

s/REPLACEME/REPLACEME

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

gibfahn commented Jul 4, 2016

Copy link
Copy Markdown
Member Author

@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 commented Jul 4, 2016

Copy link
Copy Markdown
Member

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

jasnell commented Jul 4, 2016

Copy link
Copy Markdown
Member

LGTM

mhdawson commented Jul 5, 2016

Copy link
Copy Markdown
Member

LTGM

mhdawson commented Jul 6, 2016

Copy link
Copy Markdown
Member

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

evanlucas commented Jul 6, 2016

Copy link
Copy Markdown
Contributor

LGTM

mhdawson self-assigned this Jul 6, 2016
jasnell pushed a commit that referenced this pull request Jul 6, 2016
doc: updating REPLACEME tag during release
b10ee9d 
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 commented Jul 6, 2016

Copy link
Copy Markdown
Member

Landed in b10ee9d

jasnell closed this Jul 6, 2016

rvagg commented Jul 7, 2016

Copy link
Copy Markdown
Member

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 commented Jul 7, 2016

Copy link
Copy Markdown
Member

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

addaleax commented Jul 7, 2016

Copy link
Copy Markdown
Member

@rvagg You’ve seen 2cd99eb?

rvagg commented Jul 7, 2016

Copy link
Copy Markdown
Member

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

MylesBorins mentioned this pull request Jul 11, 2016
Closed
evanlucas pushed a commit that referenced this pull request Jul 13, 2016
doc: updating REPLACEME tag during release
6c4d459 
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>
evanlucas mentioned this pull request Jul 18, 2016
Merged
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

No reviews

Assignees

mhdawson

Labels

doc Issues and PRs related to the documentations.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

Back | FazBrowse Home | New Git URL