Patch Acceptance

Moderators: Klaus, FourthWorld, heatherlaine, kevinmiller, LCMark

Post Reply
monte
VIP Livecode Opensource Backer
VIP Livecode Opensource Backer
Posts: 1561
Joined: Fri Jan 13, 2012 1:47 am
Contact:

Patch Acceptance

Post by monte » Thu Apr 11, 2013 4:19 am

The section of the contribute page titled Patch Acceptance has some (at least for me) confusing aspects. This is mainly related to the branching model that is being used and I guess I may need to see it in action to understand it. It looks like there will be some variant of git-flow employed that also has a long running maintenance branch. I'm not sure what the purpose of this branch is when tagging the master branch with release numbers and branching from a tagged commit to do a bug fix that can then be merged into both development and master seems to cover it. On a related note will there be a release branch so you can do a feature freeze during beta testing?

The other side of this is that it states that patches need to be applied to the HEAD of the branches before being submitted. Requiring it to be applied to your current HEAD seems a bit tricky in a distributed environment where we don't know how long it will take you to look at our pull request so I'm probably misunderstanding that bit. It would also be much easier if we could submit a patch from the branch we are working on so if you don't accept it we don't end up having to rewind our master or development branches.

If I want to do a bugfix I'd fork the release branch and name it with a bug number, fix it, commit, push my branch to github, send you a pull request, then you either accept or reject, if you accept you merge it in with release and possibly development then push so everyone gets it. If you reject I can make some changes to it, fetch any changes you made to release, merge the current release head into it then push again to github to have another go. If it was just a bad idea then I can just delete the branch...

Anyway, I highly recommend the following read if you haven't seen it yet: http://nvie.com/posts/a-successful-git-branching-model/
LiveCode User Group on Facebook : http://FaceBook.com/groups/LiveCodeUsers/

mwieder
VIP Livecode Opensource Backer
VIP Livecode Opensource Backer
Posts: 2820
Joined: Mon Jan 22, 2007 7:36 am
Location: Berkeley, CA, US
Contact:

Re: Patch Acceptance

Post by mwieder » Thu Apr 11, 2013 5:53 am

Agreed that submitting a pull request from a branch makes much more sense. I'll have to look at that section again to see where the disconnect is.

markw
VIP Livecode Opensource Backer
VIP Livecode Opensource Backer
Posts: 32
Joined: Mon Mar 04, 2013 4:44 pm

Re: Patch Acceptance

Post by markw » Thu Apr 11, 2013 11:59 am

There's some value in keeping a separate master branch that's only for stable releases if you use it to clean up the history and make it more readable - the head of that branch, rather than any tag, is always the latest stable release.

>> On a related note will there be a release branch so you can do a feature freeze during beta testing?
This is always a good idea. :)

I also agree about submitting pull requests from branches off of the main ones - the requirement should be that your branch is rebased to the current head at the time you submit the pull request. It's not entirely unknown / unreasonable for project maintainers to request that people rebase their pull requests again and resubmit occasionally if it's taken a long while to get to looking at them but if it happens routinely then the number of contributors soon shrinks to nothing.

LCMark
Livecode Staff Member
Livecode Staff Member
Posts: 1033
Joined: Thu Apr 11, 2013 11:27 am

Re: Patch Acceptance

Post by LCMark » Thu Apr 11, 2013 12:27 pm

This might be a result of docs that need refinement so your comments help - the model I have in my head is precisely what you suggest. Indeed, the flow is (essentially) git-flow with a facility for fixed frequency maintenance releases.

The 'master' branch will only ever contain commits (and tags) for stable releases.

The 'maintenance' branch appears after each stable release and gets merged back into 'master' for maintenance point releases. It will get patched with all accumulated maintenance patches at the start of the cycle and then immediately go into a sequence of rc's to fix regressions.

The 'development' branch is ongoing like master and is used to merge in features and more wide-ranging bug-fixes. It will be used to build dp's. When its considered stable enough for rc's, it will branch to 'release' where rc's will be generated. Finally, at the end the 'release' branch will merge into 'master'. At the point the 'development' branch forks into 'release' it will then be ready for patches for the next (non-maintenance) release.

It is the intent that everything is done in branches that get merged in. For example, if you fix a bug that is suitable to be included in a maintenance release then you branch 'master' (which will be the last stable release) and work on that to fix the bug. When ready, submit a pull-request and (if accepted) it will be merged into 'maintainance' at the next suitable point. Also, if applicable, it will get merged into 'development'.

Similarly, features should be branched off of 'development' which is where they will get merged into to (if accepted).

monte
VIP Livecode Opensource Backer
VIP Livecode Opensource Backer
Posts: 1561
Joined: Fri Jan 13, 2012 1:47 am
Contact:

Re: Patch Acceptance

Post by monte » Thu Apr 11, 2013 1:07 pm

ok, that sounds good. It sounds like maintenance is just a release branch that isn't deleted immediately after the stable release so it can be used for bugfix releases... is that right? Then you can delete it when you do the next minor or major release because there will be a release branch that's just finished the rc cycle.

BTW seeing as there's no develop branch yet I've forked master to create feature/resolveImage branch...
LiveCode User Group on Facebook : http://FaceBook.com/groups/LiveCodeUsers/

LCMark
Livecode Staff Member
Livecode Staff Member
Posts: 1033
Joined: Thu Apr 11, 2013 11:27 am

Re: Patch Acceptance

Post by LCMark » Thu Apr 11, 2013 2:09 pm

Essentially yes - maintanance forks from each stable point on master and is used to ensure all the maintainance patches don't break anything. When stable it merges back into master as something like 6.0.1 tag, then branches again for 6.0.2.

Yes - fork 'master' for now, we've got a bunch of changes we're working on to clean up some loose ends which we'll hopefully push out by early next week and that will include all the relevant branches needed for the flow.

mwieder
VIP Livecode Opensource Backer
VIP Livecode Opensource Backer
Posts: 2820
Joined: Mon Jan 22, 2007 7:36 am
Location: Berkeley, CA, US
Contact:

Re: Patch Acceptance

Post by mwieder » Thu Apr 11, 2013 5:59 pm

Thanks for the update. That sounds like a reasonable workflow plan.
I think I'm not going to branch until you're close to ready to start receiving pull requests.
For now, I'm having too much fun pawing through the codebase.




...and yes, I know I've got a weird concept of fun.

LCMark
Livecode Staff Member
Livecode Staff Member
Posts: 1033
Joined: Thu Apr 11, 2013 11:27 am

Re: Patch Acceptance

Post by LCMark » Fri Apr 12, 2013 9:52 am

Having just branched for the first maintenance release and applied the first pull request (albeit from myself!) there's a few revisions to what I said above :)

Apart from 'maintenance' being difficult to spell, there's no need to have a separate branch for it. Instead, maintenance releases will be created with a branch such as 'release-6.0.1' and it is on this branch that appropriate patches will be applied.

Bugfix patches should be prepared (whether it be by the internal team, or external contributors) on a branch bugfix-<n> from the current HEAD of master (which will be the last stable release). The bugfixes will be integrated into the release branch within a patch window, but we won't delete the branches. After the patch window closes, the release branch will only be used to apply regressions. By keeping the bugfix branches open it means they can be updated to the HEAD of the release branch to fix any regressions caused by the patch, we can then reintegrate them into the release branch during the rc phases. At the end of the cycle the bugfix branches that have been integrated can be deleted.

It will be a similar story for the development branch - the first half of the development cycle patches will be applied to development. Again, none of these branches will be deleted so that they can be rebased and any bugs relating to them fixed, then remerged. During this part dp's will be generated. The second half the development cycle will follow the same pattern as the rc stage of a maintenance release - the development branch will fork to release-6.1.0 (say), and rc's will appear from that.

After the development branch forks to release, it's then ready for patches for the next major version (e.g. 6.2).

monte
VIP Livecode Opensource Backer
VIP Livecode Opensource Backer
Posts: 1561
Joined: Fri Jan 13, 2012 1:47 am
Contact:

Re: Patch Acceptance

Post by monte » Fri Apr 12, 2013 10:11 am

I think that's just plain old git flow then with the release branch perhaps staying open a bit longer...

Sounds good... If I understand right then normally release-6.0.1 would just be release-6.0 and the .1... .X would just be merges of that to the master branch tagged with the version number... or are you thinking you would add features to minor releases and the regression stuff is just for GM1..N
LiveCode User Group on Facebook : http://FaceBook.com/groups/LiveCodeUsers/

LCMark
Livecode Staff Member
Livecode Staff Member
Posts: 1033
Joined: Thu Apr 11, 2013 11:27 am

Re: Patch Acceptance

Post by LCMark » Fri Apr 12, 2013 10:41 am

The intent is that the maintenance cycle (third point releases) would just be for 'patches that fix bugs that won't cause any backward compatibility problems nor likely to break anything else' - its a little bit of a fuzzy definition since it depends on the patch and how far reaching it is. For example, fixing a bug that causes a build to be broken (like the iOS builds in Community 6.0.0) fits that bill; as does (hopefully) fixing bug 10828 (intersect has a glitch in 6.0). This is why bugfixes should be done relative to the last stable release - when we review them we can decide if they are 'too risky' to put into the next maintenance release.

Of course, once we get continuous integration up and running, and a good set of tests running after every build, the scope of what we can include in maintenance releases increases substantially.

In terms of iterative GMs, I've never liked doing them but they have been a necessary evil at points - I'd much rather push out third-dotted maintenance releases as all changes are then nicely tracked and accounted for (and people can see more easily that its an updated version).

Post Reply

Return to “Engine Contributors”