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

build: add REPLACEME tag for version info in docs #6864

Closed
wants to merge 2 commits into from

Conversation

addaleax
Copy link
Member

Checklist
  • tests and code linting passes
  • the commit message follows commit guidelines
Affected core subsystem(s)

build

Description of change

Add a REPLACEME tag that should be used when introducing docs for new features, so that they can be updated when releases are made. Example usage:

## process.cpuUsage([previousValue])
<!-- YAML
added: REPLACEME
-->

Ref: #6578

@bnoordhuis This is taken straight from #6578 (comment) so I’ve set the commit author to you. I guess that’s okay with you?

Add a `REPLACEME` tag that should be used when introducing
docs for new features, so that they can be updated when releases
are made.

Ref: nodejs#6578
@addaleax addaleax added doc Issues and PRs related to the documentations. build Issues and PRs related to build files or the CI. labels May 18, 2016
@bnoordhuis
Copy link
Member

If you go through the trouble of filing the pull request, you can claim authorship. :-)

LGTM either way.

@addaleax
Copy link
Member Author

Credit where credit is due, I’m going to leave it like that. ;)

@claudiorodriguez
Copy link
Contributor

LGTM, should any doc be updated mentioning this?

@addaleax
Copy link
Member Author

@claudiorodriguez Hmm… got any suggestions? It might be nice to make collaborators aware of this in some way, yes.

@claudiorodriguez
Copy link
Contributor

claudiorodriguez commented May 19, 2016

@addaleax right now the added metadata is documented in /tools/doc/README.md so I think it would make sense to add this there. I'd also reference it in CONTRIBUTING.md. Maybe another PR after this one?

@bnoordhuis
Copy link
Member

Credit where credit is due, I’m going to leave it like that. ;)

Can you leave my LGTM out in that case? Otherwise I'd show up as both author and reviewer.

@addaleax
Copy link
Member Author

@bnoordhuis Of course, yep.

@claudiorodriguez I’ve added it here… that should be okay. Still LGTY?

@evanlucas
Copy link
Contributor

So, I think it was @Fishrock123 that brought it up, but we should always know that a new feature or method will be going in in the next minor right? Is this still necessary?

@addaleax
Copy link
Member Author

@evanlucas I don’t have strong feelings about it… basically, in my head it’s like this:

Pro REPLACEME:

  • People who create PRs don’t need to know when they will land/don’t have to update each time a new minor comes out, if the PR is a bit more long-running
  • Replacing REPLACEME in the docs upon release is more or less a single search+replace operation
  • (If it’s relevant, this approach may be easier if it comes to backported semver-minor changes)

Contra REPLACEME:

  • Extra work during release
  • Most of the time it one can make a good guess on the version in which a feature will land

You @nodejs/release people can probably assess things better and if you prefer not to do it this way, that’s perfectly fine.

@evanlucas
Copy link
Contributor

There are definitely pros to this approach. My biggest concern with going this route is how do we make the update permanent? Is that something the releaser should commit and push back up? Otherwise, when we hit the next release, the REPLACEME will be incorrect.

@addaleax
Copy link
Member Author

Would it be okay to do something like this together with/in the release commit? If I’ve understood everything correctly, that one is landed both in the version-specific branch and master.

@evanlucas
Copy link
Contributor

That could work. We do cherry pick the release commit from the release proposal branch back to master. So this will probably work fine for v6, but may become a little hairy for v4. I'm honestly not sure. I do think I'm +1 on this, but really think that all of @nodejs/release should see this before landing. Also, if we add this, we need to make sure that the doc/releases.md document is updated to reflect the process of updating these.

@addaleax
Copy link
Member Author

Yeah, there’s no hurry to implement this process in any case.

And regarding v4.x in general, so far this feature isn’t even implemented there, but I think I’ll do a backporting PR once we’re pretty much through with #6578.

@claudiorodriguez
Copy link
Contributor

@addaleax yep, LGTM on the PR itself. Agree on waiting on more feedback from the release team before landing though

@Fishrock123
Copy link
Contributor

Fishrock123 commented May 19, 2016

I don't really mind, but I have a question on how this will impact the release process commits-wise...

Currently we do two commits, both of which are the only un-signed-off commits people are regularly allowed to do. (Some leeway is given for if releasers mess up and have to fix links or something afterwards.)

Does this mean we add a third commit before? e.g:

  1. Update new API released: info
  2. Changelog & node_version.h (This is the tagged commit)
  3. Reset node_version.h for the next release

Or do we just roll this into the Changelog commit?

@bnoordhuis
Copy link
Member

I don't see anything wrong with rolling it into the Changelog commit.

@addaleax
Copy link
Member Author

addaleax commented Jun 5, 2016

Anybody from @nodejs/release want to give their thumbs up for this? :)

@cjihrig
Copy link
Contributor

cjihrig commented Jun 5, 2016

+1 to rolling it into the Changelog commit. And LGTM

@addaleax
Copy link
Member Author

addaleax commented Jun 5, 2016

Landed in 2cd99eb, e7f1c00. Thanks!

@addaleax addaleax closed this Jun 5, 2016
addaleax pushed a commit that referenced this pull request Jun 5, 2016
Add a `REPLACEME` tag that should be used when introducing
docs for new features, so that they can be updated when releases
are made.

Ref: #6578
PR-URL: #6864
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Claudio Rodriguez <cjrodr@yahoo.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
addaleax added a commit that referenced this pull request Jun 5, 2016
Ref: #6578
PR-URL: #6864
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Claudio Rodriguez <cjrodr@yahoo.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@addaleax addaleax deleted the doc-todos-for-added-info branch June 5, 2016 22:47
evanlucas pushed a commit that referenced this pull request Jun 15, 2016
Add a `REPLACEME` tag that should be used when introducing
docs for new features, so that they can be updated when releases
are made.

Ref: #6578
PR-URL: #6864
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Claudio Rodriguez <cjrodr@yahoo.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
evanlucas pushed a commit that referenced this pull request Jun 15, 2016
Ref: #6578
PR-URL: #6864
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Claudio Rodriguez <cjrodr@yahoo.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@evanlucas evanlucas mentioned this pull request Jun 16, 2016
addaleax added a commit to addaleax/node that referenced this pull request Jul 12, 2016
Ref: nodejs#6578
PR-URL: nodejs#6864
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Claudio Rodriguez <cjrodr@yahoo.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
addaleax pushed a commit to addaleax/node that referenced this pull request Jul 12, 2016
Add a `REPLACEME` tag that should be used when introducing
docs for new features, so that they can be updated when releases
are made.

Ref: nodejs#6578
PR-URL: nodejs#6864
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Claudio Rodriguez <cjrodr@yahoo.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jul 12, 2016
Ref: #6578
PR-URL: #6864
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Claudio Rodriguez <cjrodr@yahoo.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jul 12, 2016
Add a `REPLACEME` tag that should be used when introducing
docs for new features, so that they can be updated when releases
are made.

Ref: #6578
PR-URL: #6864
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Claudio Rodriguez <cjrodr@yahoo.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jul 12, 2016
Ref: #6578
PR-URL: #6864
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Claudio Rodriguez <cjrodr@yahoo.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jul 12, 2016
Add a `REPLACEME` tag that should be used when introducing
docs for new features, so that they can be updated when releases
are made.

Ref: #6578
PR-URL: #6864
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Claudio Rodriguez <cjrodr@yahoo.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Jul 12, 2016
MylesBorins pushed a commit that referenced this pull request Jul 14, 2016
Ref: #6578
PR-URL: #6864
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Claudio Rodriguez <cjrodr@yahoo.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jul 14, 2016
Add a `REPLACEME` tag that should be used when introducing
docs for new features, so that they can be updated when releases
are made.

Ref: #6578
PR-URL: #6864
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Claudio Rodriguez <cjrodr@yahoo.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jul 14, 2016
Ref: #6578
PR-URL: #6864
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Claudio Rodriguez <cjrodr@yahoo.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jul 14, 2016
Add a `REPLACEME` tag that should be used when introducing
docs for new features, so that they can be updated when releases
are made.

Ref: #6578
PR-URL: #6864
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Claudio Rodriguez <cjrodr@yahoo.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI. doc Issues and PRs related to the documentations.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants