-
Notifications
You must be signed in to change notification settings - Fork 79
docs on checking out a pr #892
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
Conversation
a9d2488 to
57ba966
Compare
Codecov Report
@@ Coverage Diff @@
## main #892 +/- ##
=======================================
Coverage 93.53% 93.53%
=======================================
Files 25 25
Lines 19966 19966
Branches 789 789
=======================================
Hits 18675 18675
Misses 1259 1259
Partials 32 32 Continue to review full report at Codecov.
|
grahamgower
left a comment
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.
Good idea @petrelharp. I must admit, I have to look this up every time I want to do it.
docs/development.rst
Outdated
| To do this, you first need your own local version of the git repository, | ||
| so you should first do steps 1 and 2 above. | ||
| (Strictly speaking, you don't need a fork on github | ||
| if you con't plan to edit, but it won't hurt.) |
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.
s/con't/don't/
docs/development.rst
Outdated
|
|
||
| 3. Fetch the pull request, and store it as local branch: | ||
|
|
||
| $ git fetch upstream pull/854/head:my_pr_copy |
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.
The "remote" repository will probably be called origin if they just cloned (rather than forking and following github's instructions). The output of git remote will show the possible options if one is unsure.
|
Thanks, @grahamgower - I made those edits and fixed the rendering of the code chunks. |
benjeffery
left a comment
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.
LGTM Just needs a squash.
I'm a bit wary of recapitulating large chunks of git docs found elsewhere. I was hoping github would have a doc page on doing this we could link to, but they just recommend using their gh cli tool, so I think it is worth us having this.
844aad9 to
3e9951a
Compare
|
Squashed. I was hoping to clarify not just "how to check out the pr head" but also "how to look at local changes". |
| and then navigate your web browser to the ``docs/_build/html/`` | ||
| subdirectory. | ||
|
|
||
| To test out the *code*, you can either use conda (as described above), |
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.
I'm not sure what you mean by using conda here - if you want to try out the code for the PR, it doesn't really matter whether you're using pip or conda, you still have to compile stuff?
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.
hm, right - I guess I meant that the example code below isn't using conda? but would you do just the same thing in conda?
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.
Yeah, this should all be conda/pip agnostic - they're just how you get your dev environment set up, not how you do dev.
| 3. Fetch the pull request, and store it as a local branch. | ||
| For instance, to name the local branch ``my_pr_copy``:: | ||
| $ git fetch upstream pull/854/head:my_pr_copy |
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.
Huh, I didn't know you could do this! I just do git remote add <username>, and then fetch and checkout. Your way is better for this usecase though.
As pointed out by @jeromekelleher at #892 (comment)
For instance, in #854, @yunusbb was wanting not to actually start developing, but just to try out the code from the PR. This explains how to do that (at least, how I do that?).