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: s/origin/upstream/ collaborator guide #12436

Closed
wants to merge 1 commit into from

Conversation

addaleax
Copy link
Member

Use upstream to refer to nodejs/node instead of origin, because
that’s the more common setup.

Checklist
Affected core subsystem(s)

@nodejs-github-bot nodejs-github-bot added the doc Issues and PRs related to the documentations. label Apr 15, 2017
@addaleax addaleax added the meta Issues and PRs related to the general management of the project. label Apr 15, 2017
@mscdex
Copy link
Contributor

mscdex commented Apr 15, 2017

Are you sure this works? I thought 'origin' was still the only default remote in git? If we're going to use 'upstream', then there needs to be a command showing the adding of such a remote before executing those commands.

@addaleax
Copy link
Member Author

I’m not sure what you mean by “works”… yes, calling it upstream is pure convention. But we use that name elsewhere, too, and in all of the recent onboardings (and all that I can remember) everybody had the upstream → nodejs/node, origin → username/node setup.

@benjamingr
Copy link
Member

I think origin is the more common - at the very least we should add a sentence that explains the assumption one cloned their own fork as origin and added

git remote add upstream https://github.com/nodejs/node/

to the start of "Technical HOW TO"

@mscdex
Copy link
Contributor

mscdex commented Apr 15, 2017

I guess I'm in the minority, as I've always used 'origin' to point to the original repo and then used 'github-fork' to point to my fork of the original repo. However I still think adding the line that @benjamingr shows will make it more clear, especially for first-time/new git users.

@aqrln
Copy link
Contributor

aqrln commented Apr 16, 2017

@mscdex @benjamingr we already suggest

$ git clone git@github.com:username/node.git
$ cd node
$ git remote add upstream git://github.com/nodejs/node.git

in the contributing guide, and I suppose that new collaborators start with being contributors, and all existing collaborators already know how to land PRs with or without this change :)

Besides being inconsistent with the contributing guide, the collaborator guide is even inconsistent with itself and therefore confusing (line 454 implies that PRs are opened from origin).

@Trott
Copy link
Member

Trott commented Apr 16, 2017

No opinion on how much material to add (if any) to show how to create upstream, but I will say that my experience is identical to what @addaleax says: Every collaborator I've ever onboarded had upstream set up to be the canonical repo and origin to be their GitHub fork of the repo.

Is it still a good idea to add an extra sentence or two here and a shell command example to clarify that's what's going on? I don't have an opinion on that either way.

Copy link
Member

@gibfahn gibfahn left a comment

Choose a reason for hiding this comment

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

Something like this might be more clear? LGTM anyway.

@@ -361,8 +361,8 @@ $ git checkout master
Update the tree
Copy link
Member

@gibfahn gibfahn Apr 16, 2017

Choose a reason for hiding this comment

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

Update the tree (assumes your repo is set up as detailed in CONTRIBUTING.md)

Copy link
Member Author

Choose a reason for hiding this comment

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

@gibfahn sounds good to me, done. :)

Use `upstream` to refer to `nodejs/node` instead of `origin`, because
that’s the more common setup.
@addaleax addaleax force-pushed the collab-guide-upstream branch from 29453e9 to 6a30353 Compare April 16, 2017 13:49
@gibfahn
Copy link
Member

gibfahn commented Apr 16, 2017

@mscdex LGTY now (after #12436 (comment))? I get that it's different to what you do, but at least now it's (hopefully) explicit. FWIW I never use origin for anything, because when you have more than three remotes it becomes impossible to remember which one origin was. I normally have fork and up (or DANGER if I have push access).

@refack
Copy link
Contributor

refack commented Apr 16, 2017

[silly question] can we discuss moving or linking these guides to the wiki? Personally that's the first place I look at for such information.

@gibfahn
Copy link
Member

gibfahn commented Apr 16, 2017

[silly question] can we discuss moving or linking these guides to the wiki? Personally that's the first place I look at for such information.

@refack not a silly question, but AFAIK the main reason we don't use the Wiki is that you can't PR against it, which means updates have to be done by a select few. There is stuff in the Wiki, but I don't think it's updated very often. I'd be in favour of not using it at all (or just having it be links to these docs).

@Trott
Copy link
Member

Trott commented Apr 16, 2017

AFAIK the main reason we don't use the Wiki is that you can't PR against it, which means updates have to be done by a select few.

I'm going on memory here, so I could be way off, but I thought it was a bit of the opposite problem: Anybody can update it directly and immediately, but there's no notification/watching mechanism so pages can be vandalized and we don't know until someone reports it.

Regardless of the specifics, the general picture is that the feature set on the Wiki is very limited, or at least it used to be. (I don't keep track of every new GitHub improvement and feature.) At least in the past, the Wiki was definitely not ideal for this sort of thing.

I rather like having the docs in with the code they refer to. I know for a fact that not all contributors have reliable internet and can lose access for days at a time. So it's a plus for that reason too. They can still refer to the docs as needed and work on stuff until they get back online.

@refack
Copy link
Contributor

refack commented Apr 16, 2017

AFAIK the wiki can be reflected on as a git repo (https://github.com/nodejs/node.wiki.git). And also we can limit access to people with push access
image
Is there interest in migrating some of the docs there, and maybe adding it as a git-submodule (oops I said a bad word)

Alternatively just turn it into a one page table of content, with a rudimentary explanation about who should read what.

Also maybe move some of the stuff from the root to /doc/guides (some of BUILDING.md, CHANGELOG.md, CODE_OF_CONDUCT.md, COLLABORATOR_GUIDE.md, CONTRIBUTING.md, GOVERNANCE.md)

Note to self. Make a PR of my ideas.

@gibfahn
Copy link
Member

gibfahn commented Apr 16, 2017

AFAIK the wiki can be reflected on as a git repo (https://github.com/nodejs/node.wiki.git). And also we can limit access to people with push access

Right, but you can't have PRs. The benefits for me are:

  • Non-collaborators can raise PRs against docs (a lot of Contributors get started that way).
  • Collaborators get notified about changes, so people hear about changes (e.g. new tools like node-review).
  • People can discuss changes (this whole discussion couldn't have happened in the wiki).

I don't think it makes sense to have the wiki and docs in the repo and docs on the website. The current split is:

  • Things that are useful for Node core development go in doc/guides/
  • Things that are useful for writing/running Node go on the website.

Alternatively just turn it into a one page table of content, with a rudimentary explanation about who should read what.

SGTM

@refack
Copy link
Contributor

refack commented Apr 16, 2017

Right, but you can't have PRs. The benefits for me are:

You can. you clone it into a second repo, manage issues and PRs on it, then land in main node.wiki.git. It could also be submoduled (again me with my potty mouth) or branched into the main repo for a single-clone-gets-all-docs win.

@benjamingr
Copy link
Member

@refack but then you lose the wiki part of it anyway - so making it a wiki wouldn't solve anything.

@refack
Copy link
Contributor

refack commented Apr 16, 2017

hmmm 🤔🤔🤔🤔 right
But It'll be clear where the guides are (not the API docs, which I agree should be coupled to the code)
Also it'll help deconvolute the current sitch (which I found non-trivial to start in).

@gibfahn
Copy link
Member

gibfahn commented Apr 16, 2017

@refack doesn't making the wiki link to the other docs solve the discoverability/clarity issues? If you do that, what else do you gain from having a separate repo?

I think you'd still want to put the User Info on the website, so you're basically just putting the contributor/collaborator info in a separate repo, which doesn't really make sense to me.

@refack
Copy link
Contributor

refack commented Apr 16, 2017

which doesn't really make sense to me.

Not a totally well formed idea... But I'm assuming there is good stuff in the current wiki (63 pages) that could appreciate from some loving?

Anyway I agree that curating a good ToC is more important

@gibfahn
Copy link
Member

gibfahn commented Apr 16, 2017

Not a totally well formed idea... But I'm assuming there is good stuff in the current wiki (63 pages) that could appreciate from some loving?

Sure, but again, I think that should join the stuff in the website or doc/guides, rather than the other way round.

@sam-github
Copy link
Contributor

@refack that you look at project wikis is a good argument for deleting our wiki... there is already too many repos, and lack of clarity about what goes where. We have been trying to fix this by moving to a layout where

  • the website repo has content that ends up on nodejs.org (though the node:doc/api output also lands on the website)
  • other project docs for nodejs are directly in the nodejs repo
  • and the wiki is ununused, though this is not particularly clear until you read the info in the wiki and realize how out of date it is, which someone unfamiliar with the project would not realize. Ouch.

jasnell pushed a commit that referenced this pull request Apr 17, 2017
Use `upstream` to refer to `nodejs/node` instead of `origin`, because
that’s the more common setup.

PR-URL: #12436
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@jasnell
Copy link
Member

jasnell commented Apr 17, 2017

Landed in 42d0f8b

@jasnell jasnell closed this Apr 17, 2017
@refack
Copy link
Contributor

refack commented Apr 18, 2017

@refack that you look at project wikis is a good argument for deleting our wiki... there is already too many repos, and lack of clarity about what goes where. We have been trying to fix this by moving to a layout where

Fine with me. I just like it that it's a dedicated meta-docs section in the repo.
But like I said and @gibfahn resonated, there might be good stuff there. I'll try to curate the good parts out.

P.S. vanity pages a like https://github.com/nodejs/node/wiki/Node-Users could probably just stay there?

evanlucas pushed a commit that referenced this pull request Apr 25, 2017
Use `upstream` to refer to `nodejs/node` instead of `origin`, because
that’s the more common setup.

PR-URL: #12436
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@evanlucas evanlucas mentioned this pull request May 1, 2017
evanlucas pushed a commit that referenced this pull request May 1, 2017
Use `upstream` to refer to `nodejs/node` instead of `origin`, because
that’s the more common setup.

PR-URL: #12436
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
evanlucas pushed a commit that referenced this pull request May 2, 2017
Use `upstream` to refer to `nodejs/node` instead of `origin`, because
that’s the more common setup.

PR-URL: #12436
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
gibfahn pushed a commit that referenced this pull request May 16, 2017
Use `upstream` to refer to `nodejs/node` instead of `origin`, because
that’s the more common setup.

PR-URL: #12436
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request May 18, 2017
Use `upstream` to refer to `nodejs/node` instead of `origin`, because
that’s the more common setup.

PR-URL: #12436
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins MylesBorins mentioned this pull request May 23, 2017
andrew749 pushed a commit to michielbaird/node that referenced this pull request Jul 19, 2017
Use `upstream` to refer to `nodejs/node` instead of `origin`, because
that’s the more common setup.

PR-URL: nodejs/node#12436
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.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. 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.