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

Deploying command is kind of confusing #116

Closed
agcolom opened this issue Apr 21, 2015 · 10 comments
Closed

Deploying command is kind of confusing #116

agcolom opened this issue Apr 21, 2015 · 10 comments

Comments

@agcolom
Copy link
Member

agcolom commented Apr 21, 2015

At the bottom of this section we say to use git push --tags origin master. However, everywhere else on the contribute site, we say to fork the repo, clone the fork which will be origin and add the main jquery repo as upstream. I'm proposing to replace git push --tags origin master to git push --tags upstream master. Any comments or objections?

@jzaefferer
Copy link
Member

It looks to me like the parts using "upstream" are written for contributors, the one using "origin" for maintainers. We should keep those separate, so moving that "deploying" section elsewhere would be better than renaming the remote.

@agcolom
Copy link
Member Author

agcolom commented Apr 21, 2015

@jzaefferer I agree with that. I'd be happy to maybe have a warning note then, reminding people that they should check whether they should push to origin or upstream (many maintainers started as contributors and would have the remotes inline with the contributor's recommendations. What do you think?

@jzaefferer
Copy link
Member

That makes sense.

@mgol
Copy link
Member

mgol commented Apr 21, 2015

This seems confusing to me. Why not settle on origin for everything? I always use origin for the upstream repo (this is the default) and then add my fork if needed under my GitHub name as a remote name (i.e. mzgol). Changing upstream to origin just because someone became a maintainer seems weird.

@agcolom
Copy link
Member Author

agcolom commented Apr 21, 2015

@mzgol My understanding is that contributors have been recommended to use upstream for the upstream/main repo and origin for their fork, and that is everywhere in the contribute site. I don't think we're going to change that. However, when used to follow the instructions on the contribute site, it can be confusing to suddenly be told to use origin and not upstream. While this section is for maintainers and it would be fair to expect maintainers to be aware of their setup and correct origin to upstream in that command, I think a reminder to check their remote before tagging and pushing can't do any harm and may help prevent mistakes.

@mgol
Copy link
Member

mgol commented Apr 21, 2015

Choice of specific names is personal taste so if we have upstream+origin everywhere then let's keep it. I just don't understand why we can't advise identical setup for maintainers as for contributors. After all, maintainers also submit PRs and also do it often from their own forks. I don't see the gains of separating those two cases.

For me it'd be easiest to understand if the only difference between Git setup of contributors & maintainers would be that the latter ones can push to upstream.

@scottgonzalez
Copy link
Member

For me it'd be easiest to understand if the only difference between Git setup of contributors & maintainers would be that the latter ones can push to upstream.

That's exactly what @agcolom originally suggested.

@mgol
Copy link
Member

mgol commented Apr 21, 2015

Right, sorry for the confusion. +1 to @agcolom then.

@arthurvr
Copy link
Member

👍 to changing together with adding a note to make sure people double check.

@agcolom
Copy link
Member Author

agcolom commented May 3, 2015

@jzaefferer @mzgol @scottgonzalez Please check the PR and let me know if the wording looks good to you or if anything needs changing. Here's a screenshot also:
clarifydeploy

@agcolom agcolom closed this as completed in b8f0bd4 May 4, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

5 participants