[Tutorial] How to create pull requests for core events

Discussion forum for Extension Writers regarding Extension Development.
User avatar
Wolfsblvt
Registered User
Posts: 634
Joined: Sun Oct 26, 2014 9:12 pm
Location: Solingen, Germany
Contact:

[Tutorial] How to create pull requests for core events

Post by Wolfsblvt » Thu Jan 22, 2015 6:59 pm

Okay guys. For me, this was a big deal to learn. I never had used Git before, and never contributed to phpBB.
There is many info out there for first point, even a wiki article from phpBB itself, wich is kinda outdated and over-complicated in some places.

So I have learned all needed steps and want to share them with all people who are willing to to the requests themselves, but don't know how.

This tutorial wil be step by step

How to create pull requests for core events

Step 1: Register on GitHub
This step needs to be done once.
We should start with registering on Github. If you are already registered, skip this step and just login.
Go to GitHub and enter your credentials. Sign up. You can choose "Free" as payment option and don't need to check "Help me set up an organisation next". Glick on "Finish Signup" and everything is done there. You can and should verify your account on your email address though.

Step 2: Fork phpBB repository
This step needs to be done once.
Go to the official phpBB repository and fork it. That means you will create a version of this inside your own account, wich is like your own copy :P
You have to simply click on Image in the top right corner.
It will take a few seconds, then you will have your own fork of phpBB where you can do whatever you want (:

Step 3: Set up your development environment
This step needs to be done once.
You need Git locally on your pc to do all the stuff. What tool you want to use for that depends on your operating system and your preferences. You need to install something that provides a Git Shell (this is a console for Git).
I am using GitHub for Windows wich includes a shell, so screenshots will be with this. But you can choose any other tool you want. Have a look in the topic [Info] Extension Writer Tools in section "Source Control (Git) Tools" for possible options.
If you have installed it, don't forget to login there with your GitHub account. In Github for Windows you just have to open the program and login. You can keep this window open.

When you have installed Git, you can get the forked repository to your local system. For that, just click on Image on the page that should still be open, the forked repository. It will show a window where you want to clone the repository to.
Other option is to do it with command line. Open the Shell and navigate (with command cd "path") to the folder where you want to store it. Then do the following. I have marked the important line:
Image
The link should be the one you can get on your repository under "HTTPS clone URL": Image

Greetings, you have finished setting everything up for doing pull requests!
Now comes the important part.

Step 4: Create a ticket for your event request
The first step you need to to when changing ANYTHING to phpBB is create a ticket. Nothing will be changed when there isn't a request for that. Even if you are requester and changer :P That's an important rule to keep track of development changes.
So go to phpBB Bug Tracker and login. You can use your phpbb.com login.
Click on "Create" to create a new ticket. We need to fill in the data now.
  1. Choose "Improvement" as issue type
  2. Add a short and understandable title for you ticket. For example: "Add core events on mcp_queue for approval and disapproval"
  3. Choose priority "Minor"
  4. Choose "Events" as component
  5. Affected Versions should be the latest release phpBB version where the event is still missing. So at the moment it is 3.1.2
  6. Description should be a text explaining what events you want to add and WHY you need them.
  7. Environment should contain informations about your system, but isn't really important for event requests, but more for bugs.
The form should look like this:
Image

When you are finished, click on "Create". On the popup notification you can select the link to go the ticket. It now has a number. Mine is PHPBB3-13537. You should keep that, this is very important.
The ticket is ready, so we can start changing the files now.

Step 5: Setting up the local branch for your ticket
Okay. We need to do some command line stuff now. We need to follow the steps of the wiki article in section "Bug fixing", but I'll combine them here in the tutorial, so you don't have to read it there.
First, navigate inside the local repository in the shell. You should have that still open, so you just need to type cd .\phpbb. After that, you should see a color-highlighted word behind the folder name. This is the name of the current branch: Image
We want to do the event request for 3.1, so we need "3.1.x", wich is the name of this version.
Write the following line for that: git checkout 3.1.x.
It should take a few seconds to update all files, then the last line should be with colored "3.1.x".
This is the base where we start the new code. To do that, we need the branch for our ticket.
We create it with: git branch ticket/13537, where 13537 should be your own ticket number.
To switch to the new branch we do git checkout ticket/13537.
Now your console should look like this:
Image

Step 6: Make file changes. Add the events
Okay. now you can add your events and do any changes that are needed for that. The files are inside the phpBB folder of your repository, so on my environment D:\test\phpbb\phpBB.
Just open the files inside it and adjust them with any IDE you want.
You need to take care of the following things:
  1. Add the right @since VERSION to the docblock of your event. If you don't know wich is the next released version, you can ask in webchat on channel #phpbb-dev or you can look into latest other event requests.
  2. Don't forget to add the global $phpbb_dispatcher at start of the function if it is not there already
  3. Don't break the code sniffer. No empty lines with tabs inside, no double empty lines, no $ infront of variable name in the doc block
  4. Important: If you add a style event, you have to add the description of your file manually to the file D:\test\phpbb\phpBB\docs\events.md in your repository. Just copy'n'paste a fitting description there, and make sure it is ordered alphabetically. In php events the description is automatically generated from the docblock comment
Your event may look like this:

Code: Select all

            /**
             * Perform additional actions during post(s) approval
             *
             * @event core.approve_posts_after
             * @var    string    action                Variable containing the action we perform on the posts ('approve' or 'restore')
             * @var    array    post_info            Array containing info for all posts being approved
             * @var    array    topic_info            Array containing info for all parent topics of the posts
             * @var    int        num_topics            Variable containing number of topics
             * @var bool    notify_poster        Variable telling if the post should be notified or not
             * @var    string    success_msg            Variable containing the language key for the success message
             * @var string    redirect            Variable containing the redirect url
             * @since 3.1.4-RC1
             */
            $vars = array(
                'action',
                'post_info',
                'topic_info',
                'num_topics',
                'notify_poster',
                'success_msg',
                'redirect',
            );
            extract($phpbb_dispatcher->trigger_event('core.approve_posts_after', compact($vars)));
Do any changes to any files you need to adjust.
Then continue with the next step.

Step 7: Commit your changes to your repository
Okay. we have altered the files, now we need to commit them and push them to our ticket branch in our repository.
First, we need to add all files we have changed so that Git knows what to checkin.
Use this command: git add <file>, where <file> should be path to the file name. To make it easy, you can press TAB on your keyboard, wich is suggesting folders and files for you.
In my case, the only file changed is mcp_queue.php, so I use: git add .\phpBB\includes\mcp\mcp_queue.php
Now there should be some green numbers behind your ticket branch name in the console.
For every file you add, the number in the middle should increase by one. Should look like this: Image

When all files are added, we need to commit them. The commit message should look like this

Code: Select all

[ticket/13537] Add core events on mcp_queue for approval and disapproval

Events added for the functions approve_posts(), approve_topics()
and disapprove_posts() in mcp_queue.php, so that you can send notifications
during approval and disapproval.

PHPBB3-13537
First line is the title of the checkin. We are doing normally just one checkin for an event request, so I am using the title of my ticket. Should be self explaining, just one line and shorter than 79 characters.
Then an empty line. Followed by the longer description. Just write what you have done to describe it better. Make sure, no line is longer than 79 characters!
Then an empty line, followed by your full ticket ID, with the PHPBB3 in front.
This schema is not optional, it is required!

If you are using GitHub for windows like me, it will be difficult to get the line breaks correct, so here is what I do: I write the text in a texteditor before, so that it is easily copy'n'pasteable.

Then we do the commit. To get the empty lines there is a trick. Use "-m" (the message flag) for each new block, so the whole statement looks like this:
git commit -m "[ticket/13537] Add core events on mcp_queue for approval and disapproval" -m "Events added for the functions approve_posts(), approve_topics()
and disapprove_posts() in mcp_queue.php, so that you can send notifications
during approval and disapproval." -m "PHPBB3-13537"

The easiest way (for me to insert) this is start typing git commit -m ", then I copy the header line from my text editor and paste it (with rightclick), then type " -m ", copy the description text, even with line breaks, insert it, type " -m " and paste the last block, the ticket number. Now the type the missing " and press Enter and Enter again. The commit should be done now.

Information: The steps of adding files and doing the commit messages can be simplified with Git Hooks provided by phpBB. You simply need to install them and then use git commit -a and then type the message when prompted. This will automatically add the [ticket/1111] and PHPBB3-1111 parts to your commit message and check your commit message for errors before making the commit. No need to add files or any of that other stuff either (that's what the -a is for anyways) (Thanks to VSE for this information)

After the comit (no matter wich of the two ways), you have to push the changes to GitHub.
Git tells you now what you have checked in and wich changes you made. The green numbers should be gone.
We are nearly finished with git now :P One last line is needed.
To push the commited files to your online repository, use: git push origin ticket/13537
(Remember to use your own ticket number)
Git should be finished now. It should look like this:
Image

Step 8: Create the pull request
Now that we have adjusted our repository, we need to tell phpBB that they should include it. To do that, we need a pull request. The easiest way for that is to go on the GitHub wesite again, to your forked phpBB repository. If you refresh the site, there should be an information telling you that you may want to compare and pull request your branch:
Image

Yes, we want that!
So click on the button. Now a window opens up. There needs to be a few adjustments. We need to choose 3.1.x as base branch, and we need to add the empty line before the ticket number in the discription again, wich strangely got stripped:
Image

If you have selected it, you can scroll down and see a cool comparision if you have done everything right, every change you made. If there is something wrong, you need to go back to Step 6, adjust the files, and add the changed files again with git add and all the stuff following, including the commit and push.

If everything is correct, we click Image on the right side of the comment box.

Wow, this is kinda cool. Now nearly everything is done! Your pull request is open, you can write something in the conversation there or simply wait and see if travis tests don't fail :P
But one last step is missing. Yeah, right, the ticket.

Step 9: Link your pull request as patch for the ticket
We need to tell the phpBB developers, that we made a "Fix" for this ticket. So go to the ticket in the Bug Tracker again, and click on Image.
We need to insert the url of our pull request there. Just copy it from the browser of your GitHub pull request. It looks like this: https://github.com/phpbb/phpbb/pull/3326
For the comment I just add the description text of my commit.
When you click on submit, the status of the ticket should be changed to "Awaiting Patch Review" and your pull request is added.

Now you are done. And mentally done as well, if it is the first time for you I would guess :P
Keep an eye open on your travis tests. If they go read, you may need to adjust something. Also keep an eye open on questions on your pull requests, there may will be a discussion.
Keep in mind to make your events in a way they will be useful for others too.

(Step 10:) Do additional checkins to your pull request
This step is optional.
If a developer ask you for fixes or adjustments to your pull request, or if you want to change something yourself, it is very easy to do it.
You can continue to commit and push changes to your repo like described above, just follow the lines of Steps 6-7. Any new commit should have it's on commit message, fitting the commit message rules, with ticket id, header and a description what you have changed now. After you have done the push to your repository, the pull request is updated automatically, nothing to do there. This can be repeated as often as you want, until the pull request gets merged be phpBB.

The End

If you have any suggestions for the tutorial, tell me!
Also if you have questions, this topic may be the right place as well.
Last edited by nicofuma on Wed Jun 03, 2015 12:42 pm, edited 4 times in total.
Reason: develop-* branches have been deprecated
If you have a specific extension request and you are willing to pay for, you can write me a PM.
My extensions (Trending: @Mention SystemAdvanced PollsUser Online Time)

»Du kamst zu uns. Deine Stimme kam. Du zeigtest uns die Sterne. Sie funkelten. Wir konnten sehen.«

User avatar
Wolfsblvt
Registered User
Posts: 634
Joined: Sun Oct 26, 2014 9:12 pm
Location: Solingen, Germany
Contact:

Re: [Tutorial] How to create pull requests for core events

Post by Wolfsblvt » Thu Jan 22, 2015 7:00 pm

*Reserved for additional steps/informations*
If you have a specific extension request and you are willing to pay for, you can write me a PM.
My extensions (Trending: @Mention SystemAdvanced PollsUser Online Time)

»Du kamst zu uns. Deine Stimme kam. Du zeigtest uns die Sterne. Sie funkelten. Wir konnten sehen.«

User avatar
VSE
Extensions Development Coordinator
Extensions Development Coordinator
Posts: 4571
Joined: Sat Jan 17, 2009 9:37 am
Location: Los Angeles, CA
Name: Matt Friedman
Contact:

Re: [Tutorial] How to create pull requests for core events

Post by VSE » Thu Jan 22, 2015 7:12 pm

Your commit messages are wrong. (It's why your PRs are currently failing)

The commits must be messaged with the ticket info, ie:

Code: Select all

[ticket/1111] My commit message here

PHPBB3-1111
https://wiki.phpbb.com/Git#Commit_Messages

Doing them the wrong way, you'll need to write a whole 'nuther tutorial on how to update your old commit messages so phpBB will accept the PRs :P
Official phpBB Extensions My Extensions & MODs
Please do not PM me for support.
Dictated but not read.

User avatar
Wolfsblvt
Registered User
Posts: 634
Joined: Sun Oct 26, 2014 9:12 pm
Location: Solingen, Germany
Contact:

Re: [Tutorial] How to create pull requests for core events

Post by Wolfsblvt » Thu Jan 22, 2015 7:15 pm

VSE wrote:Your commit messages are wrong.

Doing them the wrong way, you'll need to write a whole 'nuther tutorial on how to update your old commit messages so phpBB will accept the PRs :P
Oh *beep*, corrected it.
Didn't remember that :shock:

I have to? Wah. Please don't tell me that.
Then I can start again with not finding any way of changing commit messages.
If you have a specific extension request and you are willing to pay for, you can write me a PM.
My extensions (Trending: @Mention SystemAdvanced PollsUser Online Time)

»Du kamst zu uns. Deine Stimme kam. Du zeigtest uns die Sterne. Sie funkelten. Wir konnten sehen.«

User avatar
VSE
Extensions Development Coordinator
Extensions Development Coordinator
Posts: 4571
Joined: Sat Jan 17, 2009 9:37 am
Location: Los Angeles, CA
Name: Matt Friedman
Contact:

Re: [Tutorial] How to create pull requests for core events

Post by VSE » Thu Jan 22, 2015 7:17 pm

Wolfsblvt wrote:
VSE wrote:Your commit messages are wrong.

Doing them the wrong way, you'll need to write a whole 'nuther tutorial on how to update your old commit messages so phpBB will accept the PRs :P
Oh *beep*, corrected it.
Didn't remember that :shock:

I have to? Wah. Please don't tell me that :D
Yep, you have to change your commit messages doing an interactive rebase and then force push the changes back to your repo, which will update the PR.

I edited my previous post and added a link to our Git message info.
Official phpBB Extensions My Extensions & MODs
Please do not PM me for support.
Dictated but not read.

User avatar
VSE
Extensions Development Coordinator
Extensions Development Coordinator
Posts: 4571
Joined: Sat Jan 17, 2009 9:37 am
Location: Los Angeles, CA
Name: Matt Friedman
Contact:

Re: [Tutorial] How to create pull requests for core events

Post by VSE » Thu Jan 22, 2015 7:19 pm

If you only have one commit, you can use git commit -amend
Official phpBB Extensions My Extensions & MODs
Please do not PM me for support.
Dictated but not read.

User avatar
VSE
Extensions Development Coordinator
Extensions Development Coordinator
Posts: 4571
Joined: Sat Jan 17, 2009 9:37 am
Location: Los Angeles, CA
Name: Matt Friedman
Contact:

Re: [Tutorial] How to create pull requests for core events

Post by VSE » Thu Jan 22, 2015 7:30 pm

You can save two steps creating a branch. When your head is on the base branch (develop-ascraeus) just type git checkout -b origin/ticket/1111. This will create the branch and take you to it in one line.

If you install our git hooks https://wiki.phpbb.com/Git#Hooks then you can simplify the commit process down to one line. When ready to commit, just type git commit -a and then type in the message when prompted. This will automatically add the [ticket/1111] and PHPBB3-1111 parts to your commit message and check your commit message for errors before making the commit. No need to add files or any of that other stuff either (that's what the -a is for anyways)

You can also mention that you an continue making new commits and changes to this branch, and anytime you push them to your repo, the PR will be updated and retested to reflect the new commits. That means, when the developers ask for fixes to your PR, don't close it and start all over. Just keep working on your branch and committing and pushing the new changes.
Official phpBB Extensions My Extensions & MODs
Please do not PM me for support.
Dictated but not read.

User avatar
Wolfsblvt
Registered User
Posts: 634
Joined: Sun Oct 26, 2014 9:12 pm
Location: Solingen, Germany
Contact:

Re: [Tutorial] How to create pull requests for core events

Post by Wolfsblvt » Fri Jan 23, 2015 1:44 pm

I have adjusted the tutorial with your suggestions and the correct syntax to get the needed empty lines in commit messages. Except I haven't added the one-lines for branch create. I like to use statements where it is clear what you are doing, and it is only one like you could save, cause normally you will start on branch "develop" when you go to the folder, so you have to switch.
Thank you for your help here (:

Dunno if I really should add a short explenation of how to adjust commit messages. Maybe it could help others? I got my tickets right now.


I would be glad if some who hasn't already done this steps several times would do this and answer if it could help :D
Or even just answer, without doing it :P
If you have a specific extension request and you are willing to pay for, you can write me a PM.
My extensions (Trending: @Mention SystemAdvanced PollsUser Online Time)

»Du kamst zu uns. Deine Stimme kam. Du zeigtest uns die Sterne. Sie funkelten. Wir konnten sehen.«

User avatar
kasimi
Extension Customisations
Extension Customisations
Posts: 2646
Joined: Sat Sep 10, 2011 7:12 pm
Location: Germany
Contact:

Re: [Tutorial] How to create pull requests for core events

Post by kasimi » Sat Jan 24, 2015 6:49 pm

Thanks Wolfsblvt, I couldn't have done it without your tutorial, very well explained. :)

Only two questions (for now):
  1. Is GitHub messing up indentations? They are correct in my editor but in the diffs at GitHub they aren't.
  2. Before creating a new branch and making changes, I pulled the most recent version, like so:

    Code: Select all

    git checkout develop-ascraeus
    git pull
    git branch ticket/...
    Is this correct? Is it even necessary?

User avatar
VSE
Extensions Development Coordinator
Extensions Development Coordinator
Posts: 4571
Joined: Sat Jan 17, 2009 9:37 am
Location: Los Angeles, CA
Name: Matt Friedman
Contact:

Re: [Tutorial] How to create pull requests for core events

Post by VSE » Sat Jan 24, 2015 8:43 pm

1. Github uses 8 spaces to display their tabs, so they always look wrong on github
2. That is right. But again, save yourself time to create a branch git checkout -b ticket/...
Official phpBB Extensions My Extensions & MODs
Please do not PM me for support.
Dictated but not read.

User avatar
gothick
Registered User
Posts: 69
Joined: Sat Apr 21, 2007 7:20 am
Name: Matt Gibson
Contact:

Re: [Tutorial] How to create pull requests for core events

Post by gothick » Mon Feb 02, 2015 12:47 pm

VSE wrote:1. Github uses 8 spaces to display their tabs, so they always look wrong on github
Incidentally: add ?ts=4 to the end of a github URL to make the tabs look right :)

User avatar
ac_roma
Registered User
Posts: 297
Joined: Thu Mar 08, 2007 2:48 pm
Location: egypt,alexandria
Contact:

Re: [Tutorial] How to create pull requests for core events

Post by ac_roma » Thu Apr 02, 2015 8:30 am

good tutorial

antispam2022
Registered User
Posts: 1
Joined: Fri Apr 17, 2015 5:01 pm

Re: [Tutorial] How to create pull requests for core events

Post by antispam2022 » Sat Apr 25, 2015 11:08 pm

Wolfsblvt wrote:Okay guys. For me, this was a big deal to learn. I never had used Git before, and never contributed to phpBB.
I also have never used Git

Code: Select all

echo $thanks * $infinity + 1;

User avatar
javiexin
Code Contributor
Posts: 1157
Joined: Wed Oct 12, 2011 11:46 pm
Location: Madrid, Spain
Name: Javier
Contact:

Re: [Tutorial] How to create pull requests for core events

Post by javiexin » Tue Jun 02, 2015 4:41 pm

For the record, it seems that now, STEP 9 above is not needed any longer. When you commit a change in the right format specified in the tutorial, the link between the GITHub commit and the Ticket in Tracker is automatic.

Thanks a lot!
-javiexin

User avatar
javiexin
Code Contributor
Posts: 1157
Joined: Wed Oct 12, 2011 11:46 pm
Location: Madrid, Spain
Name: Javier
Contact:

Re: [Tutorial] How to create pull requests for core events

Post by javiexin » Wed Jun 03, 2015 12:35 pm

What is the base branch to use for the pull requests for 3.1 as of today?

The tutorial states it is develop-ascareus, however, it is way behind 3.1.x, that seems to be more up to date.

I have tried to use develop-ascareus always, but I did some into 3.1.x, and there seems to be almost no difference...

What is the correct way to do it? Thanks,
-javiexin

Post Reply

Return to “Extension Writers Discussion”

Who is online

Users browsing this forum: No registered users and 4 guests