Mailing List Archive

Pull requests instructions may be wrong
There are instructions for installing pull requests in trac

https://code.mythtv.org/trac/wiki/GitHubPullRequests

However, if the person submitting the request had some additional
commits of his own that he made before he created the pull request
branch, they will also be merged if you follow these instructions. That
will be at odds with what is shown on github, which assumes that only
commits made after that branch was created should be merged into the
main code.

I am far from an expert on git, but I discovered this by merging Mark's
pull request into my own github repo. I got one extra commit that was
not listed in the pull request.

I suggest that merging a pull request needs to cherry-pick all of the
listed commits. Multiple commits can be converted to one commit using
the "-n" option of cherry-pick. If it was too large (dozens of commits)
a diff could be made between before and after the pull branch and that
could be applied.

Am I correct? Any comments? Should I update those instructions in trac?

On the other hand, if we are soon moving source off trac and using git
directly, maybe it should be updated then, with github instructions,
which presumably will do the right thing.

Peter

_______________________________________________
mythtv-dev mailing list
mythtv-dev@mythtv.org
http://lists.mythtv.org/mailman/listinfo/mythtv-dev
http://wiki.mythtv.org/Mailing_List_etiquette
MythTV Forums: https://forum.mythtv.org
Re: Pull requests instructions may be wrong [ In reply to ]
On 08/16/2017 01:43 PM, Peter Bennett wrote:
> There are instructions for installing pull requests in trac
>
> https://code.mythtv.org/trac/wiki/GitHubPullRequests
>
> However, if the person submitting the request had some additional commits of his own that he made before he created the pull request branch,
> they will also be merged if you follow these instructions. That will be at odds with what is shown on github, which assumes that only commits
> made after that branch was created should be merged into the main code.
>
> I am far from an expert on git, but I discovered this by merging Mark's pull request into my own github repo. I got one extra commit that was
> not listed in the pull request.
>
> I suggest that merging a pull request needs to cherry-pick all of the listed commits. Multiple commits can be converted to one commit using the
> "-n" option of cherry-pick. If it was too large (dozens of commits) a diff could be made between before and after the pull branch and that could
> be applied.
>
> Am I correct? Any comments? Should I update those instructions in trac?
>
> On the other hand, if we are soon moving source off trac and using git directly, maybe it should be updated then, with github instructions,
> which presumably will do the right thing.
>
> Peter

My comment: I missed that. Did a diff of the unlisted commit (fb135)
and the next one (4335):

1c1
< commit fb135bee18b6ec758378502db549b60179412bc9
---
> commit 4335d4027537dd7eca568b0b0ff7665e491c09d6
5,6d4
< #13084 More ttvdb fixes
<
10c8
< * Ipstream tolerate missing cache _ignored_parameters attribute
---
> * Upstream tolerate missing cache _ignored_parameters attribute

At least it looks harmless. Maybe the result of a git commit --amend?

--
Bill
_______________________________________________
mythtv-dev mailing list
mythtv-dev@mythtv.org
http://lists.mythtv.org/mailman/listinfo/mythtv-dev
http://wiki.mythtv.org/Mailing_List_etiquette
MythTV Forums: https://forum.mythtv.org