[Qgis-developer] contributing diffs

[Larry Shaffer]

Hi,

On Thu, Feb 21, 2013 at 5:03 AM, Nathan Woodrow <madmanwoo at gmail.com> wrote:

> Thanks Larry.  Just to note that there are a few shortcuts that one can
> take that I didn't have in my work follow, things like git rebase -i master
> rather then giving it the commit.  I didn't do this in the blog post in
> order to show the logic of rebasing onto a commit.  If people have better
> cleaner ways to work then I'm happy to update the post.
>
> I would also like to see a  CONTRIBUTING file made, happy to make a start
> on that if anyone wants me to.  Even if it's just my git stuff then we can
> add stuff later.
>

+1 for starting CONTRIBUTING right now and adding to it as needed. It will
hopefully help ensure pull requests are set up, ready to merge. Good for
the requester and the commit access devs having to look over lots of
requests.

Would also be good to discuss, for the requester's benefit, how to manage a
feature branch in an ongoing pull request. For example, it is common for
the requester's github branch to become outdated, no longer capable of
being auto-merged by github.com. Luckily, the pull request feature at
github.com works off of that branch in a 'live' fashion, i.e. the requester
can pull qgis upstream master branch into theirs as a rebase and do a
'force push' back up to their github repo. The pull request will then show
the branch as viable for auto-merging again.

Here's what I do in this regard (on my feature branch):
  git rebase -i HEAD~2
  (2 being 2 commits back to rebase)

Then, after finishing rebase:
  git push -f origin HEAD
  (where 'origin' is the remote name for my github.com forked repo of QGIS)

Obviously there are potential issues with doing a force push (if others
have already pulled it), but it helps in this scenario when the feature
branch is more than likely a dead end development branch, to be abandoned
or deleted after the merge into master. Those devs testing the requester's
branch can always pull to a new local branch after the requester force
pushes.

The benefit of this approach is that the pull request stays the same, with
all request comments (not sure about per line comments, though), and the
eventual commit(s) are more concise. Process should be tested for any flaws
before writing anything up, though.

Or, we can recommend always creating a new pull request in the case of a
requester's feature branch needing rebased again.

Another issue is 3-way merges. When asking a requester to rebase their
branch via pulling from master, the project is relying upon that individual
to properly 3-way merge any conflicts (potentially messing it up and
deleting parts of other, previously committed code). Not sure the best
approach there, other than carefully going over any pull request that has
been subsequently rebased.

Lastly, we might want to consider disallowing merge commits in pull
requests. Requiring only clean, rebased commits on top of current master,
or at least merge-able into master.

Regards,

Larry


- Nathan
>
>
> On Thu, Feb 21, 2013 at 6:23 AM, Larry Shaffer <larrys at dakotacarto.com>wrote:
>
>> Hi Amit,
>>
>> On Wed, Feb 20, 2013 at 12:48 PM, Amit Kulkarni <amitkulz at gmail.com>wrote:
>>
>>> Hello,
>>>
>>> Is there a good document on the git pull workflow that you follow? Is it
>>> instead ok to submit git diff against
>>> http://github.com/qgis/Quantum-GIS/ ?
>>>
>>
>> Recently, Nathan wrote up a very nice git workflow for QGIS [0]. IMHO, it
>> should be included as a 'best practice' workflow in CODING, or maybe a new
>> document named CONTRIBUTING, in the QGIS source distribution, as a
>> reference for people looking to contribute.
>>
>> Regards,
>>
>> Larry
>>
>> [0] http://nathanw.net/2013/02/05/my-qgis-git-workflow/
>>
>>
>>
>>> Thanks
>>>
>>> _______________________________________________
>>> Qgis-developer mailing list
>>> Qgis-developer at lists.osgeo.org
>>> http://lists.osgeo.org/mailman/listinfo/qgis-developer
>>>
>>>
>>
>> _______________________________________________
>> Qgis-developer mailing list
>> Qgis-developer at lists.osgeo.org
>> http://lists.osgeo.org/mailman/listinfo/qgis-developer
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osgeo.org/pipermail/qgis-developer/attachments/20130221/03d8647f/attachment.html>