GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — doc: s/origin/upstream/ collaborator guide by addaleax · Pull Request #12436 · nodejs/node

Conversation

@addaleax

@addaleax addaleax commented Apr 15, 2017

Copy link
Copy Markdown
Member

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

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

refack reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@nodejs-github-bot nodejs-github-bot added the doc Issues and PRs related to Node.js documentation. 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

mscdex commented Apr 15, 2017 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

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.

benjamingr reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@addaleax

addaleax commented Apr 15, 2017

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@benjamingr

benjamingr commented Apr 15, 2017

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@mscdex

mscdex commented Apr 15, 2017 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

aqrln
aqrln approved these changes Apr 16, 2017
View reviewed changes
@aqrln

aqrln commented Apr 16, 2017

Copy link
Copy Markdown
Contributor

@mscdex @benjamingr we already suggest

$ git clone [email protected]: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 and lpinca reacted with thumbs up emoji
All reactions
  • 👍 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Trott

Trott commented Apr 16, 2017

Copy link
Copy Markdown
Member

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

lpinca
lpinca approved these changes Apr 16, 2017
View reviewed changes
cjihrig
cjihrig approved these changes Apr 16, 2017
View reviewed changes
gibfahn
gibfahn approved these changes Apr 16, 2017
View reviewed changes

@gibfahn gibfahn left a comment

Copy link
Copy Markdown
Member

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.

Something like this might be more clear? LGTM anyway.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread COLLABORATOR_GUIDE.md Outdated

@gibfahn gibfahn Apr 16, 2017 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

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.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

addaleax reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

@addaleax addaleax Apr 16, 2017

Copy link
Copy Markdown
Member Author

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.

@gibfahn sounds good to me, done. :)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

gibfahn reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
ghost
ghost approved these changes Apr 16, 2017
View reviewed changes
@addaleax
doc: s/origin/upstream/ collaborator guide
6a30353
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

gibfahn commented Apr 16, 2017

Copy link
Copy Markdown
Member

@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).

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

refack
refack approved these changes Apr 16, 2017
View reviewed changes
@refack

refack commented Apr 16, 2017

Copy link
Copy Markdown
Contributor

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@gibfahn

gibfahn commented Apr 16, 2017

Copy link
Copy Markdown
Member

[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).

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Trott

Trott commented Apr 16, 2017

Copy link
Copy Markdown
Member

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.

addaleax reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@refack

refack commented Apr 16, 2017 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@gibfahn

gibfahn commented Apr 16, 2017

Copy link
Copy Markdown
Member

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@refack

refack commented Apr 16, 2017

Copy link
Copy Markdown
Contributor

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@benjamingr

benjamingr commented Apr 16, 2017

Copy link
Copy Markdown
Member

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@refack

refack commented Apr 16, 2017

Copy link
Copy Markdown
Contributor

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@refack refack mentioned this pull request Apr 16, 2017
docs: curate wiki back into the repo #12449
Closed
@gibfahn

gibfahn commented Apr 16, 2017

Copy link
Copy Markdown
Member

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@refack

refack commented Apr 16, 2017

Copy link
Copy Markdown
Contributor

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@gibfahn

gibfahn commented Apr 16, 2017

Copy link
Copy Markdown
Member

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.

Trott, benjamingr, and sam-github reacted with thumbs up emoji refack reacted with confused emoji
All reactions
  • 👍 3 reactions
  • 😕 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@sam-github

sam-github commented Apr 17, 2017

Copy link
Copy Markdown
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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

jasnell
jasnell approved these changes Apr 17, 2017
View reviewed changes
jasnell pushed a commit that referenced this pull request Apr 17, 2017
@addaleax @jasnell
doc: s/origin/upstream/ collaborator guide
42d0f8b
Use `upstream` to refer to `nodejs/node` instead of `origin`, because
that’s the more common setup.

PR-URL: #12436
Reviewed-By: Alexey Orlenko <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@jasnell

jasnell commented Apr 17, 2017

Copy link
Copy Markdown
Member

Landed in 42d0f8b

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@jasnell jasnell closed this Apr 17, 2017
@refack

refack commented Apr 18, 2017

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

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?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

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

PR-URL: #12436
Reviewed-By: Alexey Orlenko <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@evanlucas evanlucas mentioned this pull request May 1, 2017
v7.10.0 proposal #12775
Merged
evanlucas pushed a commit that referenced this pull request May 1, 2017
@addaleax @evanlucas
doc: s/origin/upstream/ collaborator guide
15276e1
Use `upstream` to refer to `nodejs/node` instead of `origin`, because
that’s the more common setup.

PR-URL: #12436
Reviewed-By: Alexey Orlenko <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: James M Snell <[email protected]>
evanlucas pushed a commit that referenced this pull request May 2, 2017
@addaleax @evanlucas
doc: s/origin/upstream/ collaborator guide
077187e
Use `upstream` to refer to `nodejs/node` instead of `origin`, because
that’s the more common setup.

PR-URL: #12436
Reviewed-By: Alexey Orlenko <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@gibfahn gibfahn added the land-on-v6.x label May 16, 2017
gibfahn pushed a commit that referenced this pull request May 16, 2017
@addaleax @gibfahn
doc: s/origin/upstream/ collaborator guide
92451a7
Use `upstream` to refer to `nodejs/node` instead of `origin`, because
that’s the more common setup.

PR-URL: #12436
Reviewed-By: Alexey Orlenko <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this pull request May 18, 2017
@addaleax @MylesBorins
doc: s/origin/upstream/ collaborator guide
26fcc7a
Use `upstream` to refer to `nodejs/node` instead of `origin`, because
that’s the more common setup.

PR-URL: #12436
Reviewed-By: Alexey Orlenko <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@MylesBorins MylesBorins mentioned this pull request May 23, 2017
v6.11.0 proposal #13059
Merged
andrew749 pushed a commit to michielbaird/node that referenced this pull request Jul 19, 2017
@addaleax @andrew749
doc: s/origin/upstream/ collaborator guide
ea06645
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 <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@qubyte qubyte mentioned this pull request Nov 1, 2017
tls: allow TLSSocket construction with a list of ALPNProtocols #16655
Closed
3 tasks done
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnell jasnell jasnell approved these changes
@lpinca lpinca lpinca approved these changes
@cjihrig cjihrig cjihrig approved these changes
+4 more reviewers
@refack refack refack approved these changes
@aqrln aqrln aqrln approved these changes
@gibfahn gibfahn gibfahn approved these changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

doc Issues and PRs related to Node.js documentation. meta Issues and PRs related to the general management of the project.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

12 participants

@addaleax @mscdex @benjamingr @aqrln @Trott @gibfahn @refack @sam-github @jasnell @lpinca @cjihrig @nodejs-github-bot
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.