Some help creating a PR

Discuss general development subjects that are not specific to a particular version like the versioning control system we use or other infrastructure.
Post Reply
User avatar
Ger
Registered User
Posts: 270
Joined: Mon Jul 26, 2010 1:55 pm
Location: 192.168.1.100
Contact:

Some help creating a PR

Post by Ger » Fri Sep 16, 2016 7:03 am

Hello,

I just created a PR for adding a template event: https://github.com/phpbb/phpbb/pull/4447
However, it seems to result in a huge PR with dozens of older commits I don't have anything to do with. Somehow I must have messed up. :oops:

It's the first PR for me, so probably I have missed something. Can somebody please assist me to make this a proper PR?
Above message may contain errors in grammar, spelling or wrongly chosen words. This is because I'm not a native speaker. My apologies in advance.

Senky
Extension Customisations
Extension Customisations
Posts: 283
Joined: Thu Jul 16, 2009 4:41 pm

Re: Some help creating a PR

Post by Senky » Fri Sep 16, 2016 7:49 am

That is because you try to merge your PR into 3.2.x branch, whilst new events belong to 3.1.x branch.

User avatar
Ger
Registered User
Posts: 270
Joined: Mon Jul 26, 2010 1:55 pm
Location: 192.168.1.100
Contact:

Re: Some help creating a PR

Post by Ger » Fri Sep 16, 2016 8:03 am

But new events added to 3.1 should also be added to 3.2, shouldn't they?
Above message may contain errors in grammar, spelling or wrongly chosen words. This is because I'm not a native speaker. My apologies in advance.

User avatar
Crizzo
Translations & International Support Teams Manager
Translations & International Support Teams Manager
Posts: 14
Joined: Sun Jul 14, 2013 11:57 am

Re: Some help creating a PR

Post by Crizzo » Fri Sep 16, 2016 12:44 pm

Hi,

you need to point the PR to 3.1.x branch.

And you need to setup your new branch, that one with the Patch, on base of the current 3.1.x. branch.

Regards

User avatar
Ger
Registered User
Posts: 270
Joined: Mon Jul 26, 2010 1:55 pm
Location: 192.168.1.100
Contact:

Re: Some help creating a PR

Post by Ger » Fri Sep 16, 2016 1:15 pm

Thanks. New PR created: https://github.com/phpbb/phpbb/pull/4448
Fingers crossed... :)
Above message may contain errors in grammar, spelling or wrongly chosen words. This is because I'm not a native speaker. My apologies in advance.

Senky
Extension Customisations
Extension Customisations
Posts: 283
Joined: Thu Jul 16, 2009 4:41 pm

Re: Some help creating a PR

Post by Senky » Fri Sep 16, 2016 9:49 pm

Ger wrote:
Fri Sep 16, 2016 8:03 am
But new events added to 3.1 should also be added to 3.2, shouldn't they?
They are added by routine merging of 3.1.x into 3.2.x.

User avatar
Ger
Registered User
Posts: 270
Joined: Mon Jul 26, 2010 1:55 pm
Location: 192.168.1.100
Contact:

Re: Some help creating a PR

Post by Ger » Sat Sep 24, 2016 6:23 am

I must say I'm getting a bit tired here. While all requested changes are clear, requesting, commenting, changing and reviewing takes at least three times more time than just fixing a typo and a version bump....
Above message may contain errors in grammar, spelling or wrongly chosen words. This is because I'm not a native speaker. My apologies in advance.

User avatar
Marc
Development Team Leader
Development Team Leader
Posts: 123
Joined: Thu Sep 09, 2010 11:36 am
Location: Munich, Germany

Re: Some help creating a PR

Post by Marc » Sun Sep 25, 2016 3:13 pm

Sorry for a bit of confusion I might have caused you. While this is probably rather easy for us because we're used to it, I do understand that changing small things all over and over again is somewhat tiring. That's also why I decided to fix these last few minor issues myself and help you out with that.
If you need any help with clearing up what I changed afterwards feel free to post this here or hit me up on IRC in #phpbb-dev.

User avatar
Ger
Registered User
Posts: 270
Joined: Mon Jul 26, 2010 1:55 pm
Location: 192.168.1.100
Contact:

Re: Some help creating a PR

Post by Ger » Mon Oct 03, 2016 7:56 am

NP and thanks.
Part of my irritation was also caused by me being on holiday with only a tablet to "work" on which is annoying. I hoped it was a simple and quick addition that I committed a few days before my departure, expecting it to get merged without issues.
The requested changes were clear and sound, but adding those on a tablet was probably not the best approach. Should have parked it until today. :)

But than again: I think my intention about this PR was clear. Crizz0 than bumped the version, okay. After that, he came with an extra event. Okay. After that, Marc came with the sort order. Okay. And than a typo. And again a version.
I'm used to work in a team where we fix that kind of issues while merging the code, and reporting the issues to the committer. That works a lot faster than going back-and-forth 5 times, while commenting, waiting, reviewing and that times 5.
Above message may contain errors in grammar, spelling or wrongly chosen words. This is because I'm not a native speaker. My apologies in advance.

Post Reply