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: add remote upstream #25828

Merged
merged 1 commit into from May 8, 2023
Merged

DOC: add remote upstream #25828

merged 1 commit into from May 8, 2023

Conversation

rcomer
Copy link
Member

@rcomer rcomer commented May 6, 2023

PR summary

The dev guide currently suggests that cloning your fork will automatically add the upstream remote, but I think you have to add it manually.

PR checklist

@rcomer
Copy link
Member Author

rcomer commented May 6, 2023

Renders like this

Copy link
Member

@story645 story645 left a comment

Choose a reason for hiding this comment

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

Using 'remote' as shorthand for remote repository is a holdover from previous version so won't block, but wondering if it's worth it to add a sentence clarifying what a remote repository is and that the names for things are conventions. I know, we're not aiming to provide a "how to git" tutorial but I think this can be done very cleanly.

@rcomer
Copy link
Member Author

rcomer commented May 8, 2023

"remote" is git's term and part of the command you need to use. We have the link to GitHub's Managing remote repositories, which gives a bit more detail to what these things are, and I think the fact that you pass the GitHub url makes it fairly intuitive what you are linking to. So I'm not sure what else this needs.

I do in general have a preference for brevity though, which I'm aware is just a preference. So it's possible I'm just not seeing what you're seeing!

fork, and set up the ``upstream`` remote to point to the Matplotlib main
repository (see also `Managing remote repositories <https://docs.github.com/en/get-started/getting-started-with-git/managing-remote-repositories>`__.)
Change into this directory before continuing::
current working directory and set up the ``origin`` remote to point to your own
Copy link
Member

@timhoffm timhoffm May 8, 2023

Choose a reason for hiding this comment

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

Suggested change
current working directory and set up the ``origin`` remote to point to your own
current working directory and set up a remote repository ``origin`` pointing to your own

Is this clearer? If so, same wording should be applied to upstream

Copy link
Member

Choose a reason for hiding this comment

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

I really really really like this minor change - it addresses my concerns by explaining what the words are. Can I open a follow on PR to make this change?

@tacaswell tacaswell added this to the v3.8.0 milestone May 8, 2023
@tacaswell
Copy link
Member

I think the wording is fine as it is.

@tacaswell tacaswell merged commit 1539614 into matplotlib:main May 8, 2023
20 checks passed
@rcomer rcomer deleted the set-upstream branch May 8, 2023 16:54
story645 added a commit to story645/matplotlib that referenced this pull request May 10, 2023
building off of matplotlib#25828 and @timhoffm's comment, mostly reworded/moved
things (very little if any change in length) to try and make it more
explicit that in this context origin and upstream are aliases for
remote repositories.
story645 added a commit to story645/matplotlib that referenced this pull request May 10, 2023
building off of matplotlib#25828 and @timhoffm's comment, mostly reworded/moved
things (very little if any change in length) to try and make it more
explicit that in this context origin and upstream are aliases for
remote repositories.
story645 added a commit to story645/matplotlib that referenced this pull request May 10, 2023
building off of matplotlib#25828 and @timhoffm's comment, mostly reworded/moved
things (very little if any change in length) to try and make it more
explicit that in this context origin and upstream are aliases for
remote repositories.

Co-authored-by: Tim Hoffmann <2836374+timhoffm@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants