# About merging Pull Request workflow - Thread: https://externals.io/message/86331 - Original archive: https://news-web.php.net/php.internals/86331 - Started by Remi Collet on 2015-05-21T07:16:31+00:00 - Messages: 5 - Reply by email to internals@lists.php.net - More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt --- ## #86331 — Remi Collet — 2015-05-21T07:16:31+00:00 URL: https://externals.io/message/86331#86331 -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Hi, Current workflow described in https://wiki.php.net/vcs/gitworkflow#merge_a_pull_request Problem, git history only give info about a "merge" Ex, after merging 1278: http://git.php.net/?p=php-src.git;a=commit;h=eef762d576e7744d6e0c8d6a3f997fe911b2dc34 Another simple workflow could be 1/ Download the patch https://github.com/php/php-src/pull/1278.patch 2/ apply it git am 1278.patch 3/ close the PR from https://qa.php.net/pulls This preserve credits to the authors and give a better history. What do you think ? Remi -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iEYEARECAAYFAlVdhk8ACgkQYUppBSnxahhKuACfbXzDxPsZJTyfwLtyoTALPVhs dbUAn2kk9vlD//ZQvea5Me1KU2JMgjdQ =1Xi5 -----END PGP SIGNATURE --- ## #86332 — Lior Kaplan — 2015-05-21T07:26:06+00:00 URL: https://externals.io/message/86331#86332 · In reply to: #86331 Yep, is a real problem. We should keep the credit / commit info. I think we should remove "--no-ff" so it would just add the relevant commits instead of actually merging them without history (or even rebase the pull request to recent branch before merging). Kaplan On Thu, May 21, 2015 at 10:16 AM, Remi Collet wrote: > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA1 > > Hi, > > > Current workflow described in > https://wiki.php.net/vcs/gitworkflow#merge_a_pull_request > > Problem, git history only give info about a "merge" > > Ex, after merging 1278: > > > http://git.php.net/?p=php-src.git;a=commit;h=eef762d576e7744d6e0c8d6a3f997fe911b2dc34 > > Another simple workflow could be > > 1/ Download the patch > https://github.com/php/php-src/pull/1278.patch > > 2/ apply it > git am 1278.patch > > 3/ close the PR from > https://qa.php.net/pulls > > This preserve credits to the authors and give a better history. > > What do you think ? > > > Remi > -----BEGIN PGP SIGNATURE----- > Version: GnuPG v2 > Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ > > iEYEARECAAYFAlVdhk8ACgkQYUppBSnxahhKuACfbXzDxPsZJTyfwLtyoTALPVhs > dbUAn2kk9vlD//ZQvea5Me1KU2JMgjdQ > =1Xi5 > -----END PGP SIGNATURE --- ## #86333 — Stanislav Malyshev — 2015-05-21T07:27:54+00:00 URL: https://externals.io/message/86331#86333 · In reply to: #86331 Hi! > Current workflow described in > https://wiki.php.net/vcs/gitworkflow#merge_a_pull_request > > Problem, git history only give info about a "merge" If you merge from a pull, the history should contain both original commit and the merge commit. So you know both who created the patch and who merged it. At least for me it always did. Unless you use some special tricks to edit the history, such as --no-commit. > 1/ Download the patch > https://github.com/php/php-src/pull/1278.patch > > 2/ apply it > git am 1278.patch > > 3/ close the PR from > https://qa.php.net/pulls You can do this but it's more work and tries to work around github instead of using facilities it provides. I'm not sure there's a reason for that. -- Stas Malyshev smalyshev@gmail.com --- ## #86334 — Anatol Belski — 2015-05-21T07:32:20+00:00 URL: https://externals.io/message/86331#86334 · In reply to: #86331 Hi, > -----Original Message----- > From: Remi Collet [mailto:remi@fedoraproject.org] > Sent: Thursday, May 21, 2015 9:17 AM > To: internals@lists.php.net > Subject: [PHP-DEV] About merging Pull Request workflow > > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA1 > > Hi, > > > Current workflow described in > https://wiki.php.net/vcs/gitworkflow#merge_a_pull_request > > Problem, git history only give info about a "merge" > > Ex, after merging 1278: > > http://git.php.net/?p=php- > src.git;a=commit;h=eef762d576e7744d6e0c8d6a3f997fe911b2dc34 > > Another simple workflow could be > > 1/ Download the patch > https://github.com/php/php-src/pull/1278.patch > > 2/ apply it > git am 1278.patch > > 3/ close the PR from > https://qa.php.net/pulls > > This preserve credits to the authors and give a better history. > > What do you think ? > > > Remi > -----BEGIN PGP SIGNATURE----- > Version: GnuPG v2 > Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ > > iEYEARECAAYFAlVdhk8ACgkQYUppBSnxahhKuACfbXzDxPsZJTyfwLtyoTALPVhs > dbUAn2kk9vlD//ZQvea5Me1KU2JMgjdQ > =1Xi5 > -----END PGP SIGNATURE----- > > -- > To unsubscribe, visit: > http://www.php.net/unsub.php Even more would make sense to do a rebase so the commits from the PR are put on the top of the branch. Not sure "git am" would trigger that. When one could avoid the merge commit, the history were of course cleaner. I never looked inside that merge mechanics, but in general - such an automation, say when you only need to merge and the PR is marked automatically, is probably a good thing. Well, sometimes it fails but for that we've that special tool. Regards Anatol --- ## #86336 — Anatol Belski — 2015-05-21T08:00:23+00:00 URL: https://externals.io/message/86331#86336 · In reply to: #86333 > -----Original Message----- > From: Stanislav Malyshev [mailto:smalyshev@gmail.com] > Sent: Thursday, May 21, 2015 9:28 AM > To: Remi Collet; internals@lists.php.net > Subject: Re: [PHP-DEV] About merging Pull Request workflow > > Hi! > > > […] > > If you merge from a pull, the history should contain both original commit and > the merge commit. So you know both who created the patch and who merged > it. At least for me it always did. Unless you use some special tricks to edit the > history, such as --no-commit. > I've just made an exercise as I wanted to backport that PR to 5.5. So I went by Remi's way. As "git am" worked cleanly, I've just pushed it. Result - the original author credit is kept, the person who did "git am" is the commiter, no merge commit. Plus, IMO a big plus - the commits from the PR are put on the top. Say if there's a branch with commits made a year ago, those commits won't be seen with the current approach from the wiki. They'll be possibly too far in the history. So all in all, Remi's suggestion sounds eligible. Credits held, commiter is recorded, cleaner history and commits on the top as a bonus :) If we could integrate it with the current github automatics, that were an improvement, IMHO. Regards Anatol