Jump to content

Pull Request Q&A


Maki

Recommended Posts


  • Group:  Members
  • Topic Count:  146
  • Topics Per Day:  0.03
  • Content Count:  1195
  • Reputation:   467
  • Joined:  11/15/11
  • Last Seen:  

Fork & Pull Request on rAthena

 

>>>> Fork & Make New Pull Request <<<<

 

>>>> How to update your fork <<<<

 

 




 

Introducing (a short guide to) GitHub Pull Requests

 

 

Submitting a Pull Request

 

1.  Once you've commited and squashed your changes, push them to your remote like this:

git push origin newfeature 

2.  Then go to that page on GitHub and change branches to the one for your new feature.

 

aAd2v.png

Submit a Pull Request!

 

 

3.  Then, click on the little button that says 'Pull Request'. This will bring you to a page asking you to describe your change. Describe it thoroughly.

5Euiy.png
Describe your Pull Request.

 

 

4.  Then press 'Submit Pull Request'.

 

 

Now, you're not quite done yet. If the maintainer finds some problems with your code, they won't want to pull your changes until you fix them. Fortunately, whenever you commit and push more things to that branch of your code, they will be included in that pull request until it is closed.

 

 

Accepting And Merging a Pull Request

 

If you're on the receiving end of a pull request, how do you merge the changes?

Easy - press the button! GitHub now has an auto-merge button which does everything for you in one click. However, it doesn't always work, in which case you'll have to do the merge on your own machine, like so:


And then their changes will be properly merged into your main master branch.

 

 

So, Your Pull Request Was Rejected. Now What?

Qyyh5.gif
Some forks are unavoidable.

 

Sometimes, for technical or organizational reasons, your pull request will be rejected. This can feel really frustrating, and there are a few different ways you can proceed. Just remember to act rationally.

 

The simplest thing is to simply accept their judgement. It's their project, and a good maintainer knows when to say "no." If their argument is logically sound, you should accept it. If you don't think it's a particularly important feature, hopefully whoever is looking at the project will check the Network and Issues tabs of the upstream project and will notice your changes.

 

 


 

Easy to follow video

http://www.youtube.com/watch?v=-CpllxXPsvw

 

 

ELI5

http://www.youtube.com/watch?v=4ocbq8vx4vI

 

 

Other Resources

https://help.github.com/articles/using-pull-requests

Original Article

 


 

If you have any questions, please feel free to ask!  We will try and assist you as best as we can.  Users who are knowledgeable with GitHub Pull Requests are also welcome to help! tyranitar_icon_by_revois-d5ayaeh.gif

Edited by Cydh
Just point the post that gives 'how to fork and do PR' for rAthena
  • Upvote 2
  • Like 1
Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  81
  • Topics Per Day:  0.02
  • Content Count:  1654
  • Reputation:   583
  • Joined:  08/09/12
  • Last Seen:  

A noob-friendly tutorials will be appreciated  /ok

Link to comment
Share on other sites


  • Group:  Developer
  • Topic Count:  48
  • Topics Per Day:  0.01
  • Content Count:  1443
  • Reputation:   337
  • Joined:  10/17/12
  • Last Seen:  

A noob-friendly tutorials will be appreciated  /ok

Ya this would be awesome, also with git i can add my own custom releases in a fork right? I wonder how, I'm to lazy to look up atm maybe ill look later if no one answers~

Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  146
  • Topics Per Day:  0.03
  • Content Count:  1195
  • Reputation:   467
  • Joined:  11/15/11
  • Last Seen:  

A noob-friendly tutorials will be appreciated  /ok

Ya this would be awesome, also with git i can add my own custom releases in a fork right? I wonder how, I'm to lazy to look up atm maybe ill look later if no one answers~

 

Yes that would be possible!  I split the the above two posts from the original topic (so we did not get too off-topic).

Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  4
  • Topics Per Day:  0.00
  • Content Count:  197
  • Reputation:   13
  • Joined:  05/14/12
  • Last Seen:  

Awesome maki thanks really help :)

Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  42
  • Topics Per Day:  0.01
  • Content Count:  1096
  • Reputation:   344
  • Joined:  02/26/12
  • Last Seen:  

http://git-scm.com/ git tutorial. Dear RO servers must read & use it for quality coding.

Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  81
  • Topics Per Day:  0.02
  • Content Count:  1654
  • Reputation:   583
  • Joined:  08/09/12
  • Last Seen:  

Thanks Maki for your reply! /ok

 

So what I have to do?

  1. Make a fork from 'master' branch.
  2. 'Git Clone' it to my local drive.
  3. Make changes on local drive.
  4. 'Push' changes to my fork repo.
  5. Submit 'Pull Request' to Master branch.

Is there any step that I miss?

Link to comment
Share on other sites


  • Group:  Developer
  • Topic Count:  153
  • Topics Per Day:  0.04
  • Content Count:  2285
  • Reputation:   745
  • Joined:  06/16/12
  • Last Seen:  

I got this problem when tried to clone the "forked" rAthena.

####################> git clone -v --progress https:/
/github.com/cydh/rathena
Cloning into 'rathena'...
POST git-upload-pack (250 bytes)
remote: Counting objects: 102175, done.
remote: Compressing objects: 100% (16110/16110), done.
effatal: The remote end hung up unexpectedlyMiB | ##.## KiB/s
atal: early EOF
rror: RPC failed; result=18, HTTP code = 200
fatal: index-pack failed

 
Solved with this command

git config --global http.postBuffer 524288000

NOTE, if u're using windows, maybe u need to find correct directory with 'git.exe' and use 'git.exe' syntax instead 'git'

 

Git clone return result=18 code=200 on a specific repository

  • Upvote 1
Link to comment
Share on other sites

  • 2 months later...

  • Group:  Developer
  • Topic Count:  153
  • Topics Per Day:  0.04
  • Content Count:  2285
  • Reputation:   745
  • Joined:  06/16/12
  • Last Seen:  

Make sure when you read this, open this photo album, source
 
How to fork rAthena?

  • You must have GitHub account.
  • Open rAthena repo on github: https://github.com/rathena/rathena
  • On top right-corner, you will see "Fork" button, click it (Pic. 0.jpg)
  • Then choose your account to place for forked rAthena. (That pic, mine, show @cydh and @rathena because I had been invited to rAthena repo itself) (Pic. 1.jpg)
  • Well it's done!

Clone your forked rAthena:
Here for Windows user, I used SmartGit. :P

  • Project -> Clone
  • Input the Repository URL, it should be https://github.com/<yourusername>/rathena (Pic. 2.jpg)
  • Just "Next" /heh (Pic. 3.jpg)
  • Choose your local directory (Pic. 4.jpg)
  • GIve the Project Name (Pic. 5.jpg)
  • Well it clone your forked rAthena, just wait (Pic. 6.jpg)

How to update my forked rAthena?
 
First, you need make sure your forked repo is up-to-date with rAthena repo. For this steps, use this source images
(Step: 1~8, only for the first time)

  • Click Remote menu -> Add.. Fill the URL or Path with https://github.com/rathena/rathena and give the name "rAthena" (1.jpg)
  • After the remote repo is created, right-click it -> Fetch More... (2.jpg)
  • Choose the master branch. (3.jpg)
  • If any changes, it will fetch them. (4.jpg)
  • Make sure your active branch is your local master.
  • Right-click on the master branch of rAthena remote, choose Merge (5.jpg)
  • Click Fast-Forward (6.jpg)
  • Well, is it already up to date? (7.jpg) If yes, do nothing. /heh
  • Make sure your active branch is your local master.
  • For another day maybe, you can simply right-click on rAthena remote choose Pull (10.jpg)
  • Click Fetch Only (11.jpg), and wait until it done (12.jpg)
  • Then you need merge it to your local, right-click on master branch of rAthena remote, choose Merge... (13.jpg)
  • Fast-Forward (14.jpg) (Only if your local doesn't have conflict it the 'incoming' fetch, if there is conflict, it will be Create "Merge Commit" and you should solve the conflict)
  • And you will get this log (something like this) (15.jpg) That's mean, it's done
  • Push them to your repo on GitHub! Just click Push (16.jpg), then just choose the branch master only (17.jpg)
  • Wait it, and see the changes when it's done. (18.jpg)

 

 

 

Before make Pull Request (PR)

Better you make new branch, keep master branch SAME with rAthena master branch. Why?
- It won't disturb your or rAthena master branch
- 1 branch 1 PR, so you can make many PR for each branch.
- Merged/closed/rejected branch can be deleted, so if your PR from non-master branch is rejected, your master branch still same, you can make PR again later without your prev. rejected branch disturb your next PR.
- Make new branch on your github, just go to your fork, the type branch name, if the branch isn't exist, it will offers to make new one, example "test" branch, then pull your repo first.
See this guide how to do it!

How to do Pull Request?
(Back to this album again)
  • Edit the file you want (make sure everything is correct and working properly)
  • Open your SmartGit, open the project.
  • Make/switch branch, Branch menu > Add Branch (F7), name it as "test" for example.
  • (Better you click the "Directories" first on the left, then) Click "Commit" button
  • A Commit window will appears, (choose the file taht you want to commit & push)
  • Give commit message. Then click "Commit & Push" (Pic. 7.jpg)
  • Wait until it done! (Pic. 8.jpg)

Here is it, do a Pull Request!

  • Open your browser and open your forked repo (mine is http://github.com/cydh/rathena)
  • Then click "compare" (pic. 9.jpg) or you can use
    https://github.com/<username>/<rathena fork name>/compare/rathena:master...<branch name>
  • Click "Click to create a pull request for this comparison" (pic. 10.jpg)
  • Give some messages, then click "Send pull request" (pic. 11.jpg)
  • Done.
  • Upvote 6
Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  40
  • Topics Per Day:  0.01
  • Content Count:  587
  • Reputation:   104
  • Joined:  11/19/11
  • Last Seen:  

I m using  Tortoisegit /oops   How to update my forked rAthena? XD

Link to comment
Share on other sites


  • Group:  Developer
  • Topic Count:  153
  • Topics Per Day:  0.04
  • Content Count:  2285
  • Reputation:   745
  • Joined:  06/16/12
  • Last Seen:  

I m using  Tortoisegit /oops   How to update my forked rAthena? XD

the first step for u is, "install SmartGit" /heh

 

EDIT:

 

How to update fork by using TortoiseGit

See this album

  1. Right-click your local fork, choose Settings
  2. On the left menu-list, click on Git -> Remote. (pic)
  3. Need to new remote for rAthena, just type 'rAthena' on remote textfield (it gives the remote name) and the URL is https://github.com/rathena/rathena.git (pic)
  4. Click Add New/Save. U will get new remote, named rAthena. (pic)
  5. After adding the new remote, there is prompt "Do you want to fetch remote branches from newly added remote?" Yes! (pic)
  6. Look at this picture, and OK
  7. Wait until it done, you will see some it works, fetching from rAthena. (pic)
  8. If it's done, right-click on your local fork, choose Merge,
  9. Select remotes/rAthena/master on the Branch from (look at this picture), then OK. (pic)
  10. Wait until it done (it should be fast, it just merging ur local fork). (pic)
  11. Then u need to push it. Right-click on ur local repo, choose Push.
  12. On the Ref, select for local is ur master, and Remote is master too. And the Destination Remote is origin (it will be pointed to ur https://github.com/<username>rathena. OK. (pic)
  13. Wait it done, and look by browser on ur repo, it should be no different between urs and rAthena's repo. (pic)

*bump*

  • Upvote 3
Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  40
  • Topics Per Day:  0.01
  • Content Count:  587
  • Reputation:   104
  • Joined:  11/19/11
  • Last Seen:  

 What an awesome guide! dunno why I prefer Tortoisegit, really help a lot. /no1

 

  So, After first updated , What should i do  is  fetching from remote(rAthena) --> Merge local master -->

   push remote(origin ) , Is it correct?

Edited by QQfoolsorellina
Link to comment
Share on other sites


  • Group:  Developer
  • Topic Count:  153
  • Topics Per Day:  0.04
  • Content Count:  2285
  • Reputation:   745
  • Joined:  06/16/12
  • Last Seen:  

 What an awesome guide! dunno why I prefer Tortoisegit, really help a lot. /no1

 

  So, After first updated , What should i do  is  fetching from remote(rAthena) --> Merge local master -->

   push remote(origin ) , Is it correct?

yup

Link to comment
Share on other sites

  • 1 month later...

  • Group:  Members
  • Topic Count:  218
  • Topics Per Day:  0.05
  • Content Count:  1180
  • Reputation:   141
  • Joined:  01/27/12
  • Last Seen:  

I downloaded TortoiseGIT and installed it. I however can't figure out how commit the git files using it. I can right click on a new folder and go to GIT Clone. But it pops up with a dialog box that says Git for Windows (http://code.google.com/p/msysgit/) not found. I have tried the options for Set Git path with no success. Is there a config files I missed for it or something I'm doing wrong? Thanks.

 

Peopleperson49

Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  81
  • Topics Per Day:  0.02
  • Content Count:  1654
  • Reputation:   583
  • Joined:  08/09/12
  • Last Seen:  

You need to install msysgit first, Peopleperson49

After that you need to fork from master repo to make your own fork repo, from there you can change the files on your local disk and later you can commit & push to your fork repo.

When you feel that your commits are done, then you can make a Pull Request into the master repo

Just be sure to Pull/Fetch before you edit the files on your local, because if you forgot to do that, there's a chance that your commit will got conflicted with the current branch  /ok

  • Upvote 1
Link to comment
Share on other sites

  • 5 weeks later...

  • Group:  Members
  • Topic Count:  218
  • Topics Per Day:  0.05
  • Content Count:  1180
  • Reputation:   141
  • Joined:  01/27/12
  • Last Seen:  

I'm trying figure out how to do a pull request, but even with this topic I'm lost. I am at the page https://github.com/rathena/rathena/pulls and it shows me who has made them and their status, but nothing about letting me add. I have tried to use TortoiseGIT (with msysgit installed), but I get errors such as Invalid git.exe path. It says check help file, but that don't seem to help either. I am able to download the github for rathena, but from that point on its not working. What is it I'm missing here? Thanks.

 

Peopleperson49

Edited by Peopleperson49
Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  81
  • Topics Per Day:  0.02
  • Content Count:  1654
  • Reputation:   583
  • Joined:  08/09/12
  • Last Seen:  

If you want to make a 'Pull Request' to the master repository, you have to make your own 'Fork', then you make your edit on your fork..

Link to comment
Share on other sites

  • 3 months later...

  • Group:  Developer
  • Topic Count:  153
  • Topics Per Day:  0.04
  • Content Count:  2285
  • Reputation:   745
  • Joined:  06/16/12
  • Last Seen:  

better make new branch for your fork before making pull request

Link to comment
Share on other sites

  • 2 months later...

  • Group:  Developer
  • Topic Count:  153
  • Topics Per Day:  0.04
  • Content Count:  2285
  • Reputation:   745
  • Joined:  06/16/12
  • Last Seen:  

How to make a new branch for PR?

  • Go to your rAthena fork on your github account, https://github.com/<yourusername>/rathena
  • Make sure, you're in "master" branch first, so it will makes new branch based on "master"
  • Make new branch by click that "branch" and just type branch name that you want. Example "idea" (thank anubisK that inspired a cool name for sample branch) post-5421-0-47826300-1407672363_thumb.png
  • Branch created. post-5421-0-16188000-1407672370_thumb.png
  • Pull your local first, you will get this notif about latest pull contains new branch. post-5421-0-06542800-1407672376_thumb.png
  • Now, make new branch on your local, make sure the active branch is "master". post-5421-0-63667300-1407672380_thumb.png
  • Add new branch with same name "idea". post-5421-0-44154400-1407672386_thumb.png post-5421-0-79523200-1407672390_thumb.png
  • New branch created. Now set the tracked branchpost-5421-0-48681100-1407672393_thumb.png post-5421-0-77427500-1407672397_thumb.png post-5421-0-61812500-1407672401_thumb.png
  • Well, now u're in "idea" branch, you can edit something. I tried to edit README.txt and README.md. Save your changes.
  • Commit & Push it, make sure your changes in "idea" branch not in master branch.
  • Give a little log messages. post-5421-0-14242300-1407672406_thumb.pngpost-5421-0-94674300-1407672435_thumb.png
  • Now, go to Github again, look at your new branch commits. Mine, https://github.com/cydh/rathena/commits/idea. OK nothing to here, I just want to show you the changes. post-5421-0-07112100-1407672440_thumb.png
  • Now, browse source on idea branch. Like mine, https://github.com/cydh/rathena/tree/idea
  • You can click "compare" or "compare & pull request", or by enter this URL https://github.com/cydh/rathena/commits/idea post-5421-0-63273700-1407672444_thumb.pngpost-5421-0-94801400-1407672447_thumb.png
  • After done you comparing, hit that "Create pull request"! post-5421-0-74626300-1407672452_thumb.png
  • Give a little notice maybe? "Create pull request!post-5421-0-04404700-1407672458_thumb.png
  • OK, your new Pull Request is ready, you can wait until it merged or rejected.
  • Once it merged or rejected, you can delete the "idea" branch, it won't bothers your "master" branch fork. post-5421-0-56956600-1407672464_thumb.png
  • Then, for sync your master branch, just do like on my other guide how to update forked repo.

You should do those steps before making new PR, because this won't mess your forked repo & the master branch's commits. Just try asked someone who already mess their fork, he/she have to reset their fork (delete current fork and refork) to make their "master" branch clear.

  • Upvote 2
Link to comment
Share on other sites

  • 2 years later...

  • Group:  Members
  • Topic Count:  4
  • Topics Per Day:  0.00
  • Content Count:  399
  • Reputation:   69
  • Joined:  12/26/15
  • Last Seen:  

any guide by using github software? i try to follow the guide provided. but got stuck in middle halfway.

https://github.com/rathena/rathena/blob/master/.github/CONTRIBUTING.md

How to create Pull Requests ??

1. Make sure you have a GitHub account.
2. Next, you will need to fork rAthena to your account.
3. Before making changes, make sure you create a new branch for your working tree.
4. After completing your changes, commit and push it to your branch.
5. Now you are ready to create a Pull Request for rAthena!

As on number 4. I already commit, which in the new branch. so, here's the question. should i commit in new branch or commit in the master branch.? what i did is i commit in the new branch. upon commit in the new branch, I create the PR.. but, somehow, my PR have checks failed.

this is the failed message. Add more commits by pushing to the expanded-baby-job branch on hazimjauhari90/rathena.

but, another question i would like to ask, how would it should be done. commit new branch & push to master branch. is that so?

anyway, sorry for a such very simple & basic question. any guide & help are much appreciate. thank you.

Link to comment
Share on other sites

  • 8 months later...

  • Group:  Members
  • Topic Count:  2
  • Topics Per Day:  0.00
  • Content Count:  7
  • Reputation:   1
  • Joined:  01/16/12
  • Last Seen:  

Hi, thanks for the guide.

I'm new to this pull request thing, if my pull request is approved and merged, should i close the issue related to it? or just wait to be closed by the receiving end?

Link to comment
Share on other sites


  • Group:  Developer
  • Topic Count:  153
  • Topics Per Day:  0.04
  • Content Count:  2285
  • Reputation:   745
  • Joined:  06/16/12
  • Last Seen:  

On 12/4/2017 at 9:07 AM, uddevil said:

Hi, thanks for the guide.

I'm new to this pull request thing, if my pull request is approved and merged, should i close the issue related to it? or just wait to be closed by the receiving end?

yes you should. or rather, devs will take care the closing action for every opened issue (except you found that your issue is invalid)

later if you make another PR for fixing something based on Issue, please use "Fixes #nnnn" so it'll be auto-closed

Edited by Cydh
  • Upvote 1
Link to comment
Share on other sites

  • 2 years later...

  • Group:  Members
  • Topic Count:  3
  • Topics Per Day:  0.00
  • Content Count:  25
  • Reputation:   17
  • Joined:  12/12/15
  • Last Seen:  

What is the best way to help approved pull requests (not on hold) get merged?
For example this filtered list of pull requests: https://github.com/rathena/rathena/pulls?q=is%3Apr+is%3Aopen+review%3Aapproved+-label%3Astatus%3Aonhold

Would adding things such as screenshots or a video of testing help?

I suppose I'm wondering how to help aspiring contributors understand what work is remaining on a PR before it gets merged.
It'd be helpful to know if we can do anything else to assist devs for a given PR.

Link to comment
Share on other sites

  • 9 months later...

  • Group:  Members
  • Topic Count:  90
  • Topics Per Day:  0.02
  • Content Count:  361
  • Reputation:   18
  • Joined:  01/09/13
  • Last Seen:  

I don't know where to ask, but are we not posting Digestive Updates anymore? /heh 

Link to comment
Share on other sites

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Paste as plain text instead

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

×
×
  • Create New...