Guideline for Development Workflow

[Gary Cao]

Hi guys,

We had a lot of issues with the last release (3.8.7.6.1) that cause us to rethink our development process and quality control. I feel that it has been my responsibility for not putting together a clear guideline sooner, which leads to a lot of confusion regarding the branches, how to commit, and how to ensure quality control.

Better late than never. I'd like to post my suggestions about how the development process should be from now on (3.8.9).

Current State

Google Code vs. Github

Currently we're using Google Code as the primary project management platform. We also have a mirror on github (http://github.com/instinct/wp-e-commerce). However, Google Code is the only place where issue tickets are monitored and commits go in. If you like git and github, feel free to fork the instinct wpec github above. If you are not familiar with git or github, just ignore it. Until further instructed, everything important takes place in Google Code.

Branches

Currently we have a few branches that are actively maintained:

• trunk: This is where 4.0 development resides. New features and bug fixes can go into this branch.
• branch-3.8.7: This is where 3.8.7-dev resides. Because the spec is already frozen for 3.8.7, only bug fixes can go in.
• branch-3.8.8: This is where 3.8.8-dev resides. Because the spec is already frozen for 3.8.8, only bug fixes can go in.
• branch-3.8: This is where 3.8-dev resides. This is the bleeding-edge development snapshot of the 3.8 branch (currently for 3.8.9 development). Both new features as well as bug fix can be committed to this branch.
  

Why the hell are there so many branches?

To answer this, we need to understand about backward-compatibility, major and minor releases.

Release Cycle and Frozen State

Backward-compatibility means themes or plugins that depend on the previous major release don't have to change their code to accomodate for architectural changes in the new major release.

Major release: Releases that contain new features, enhancements, bug fixes or architectural changes. Major releases are allowed to break backward-compatibility, however we try to minimize that as much as we can.

Examples: the upcoming WPEC 3.8.8 is a major release that contains various enhancements in UI. The upcoming WPEC 4.0 is also a major release that contains fundamental architectural changes related to theme engine and payment gateways.

Minor release: Releases that only contain bug fixes introduced in previous major release. Examples: WPEC 3.8.7.1 contains the same feature set as 3.8.7, but it has some more bug fixes in it.

Mini release: Releases that fixes bugs introduced in minor release. Examples: WPEC 3.8.7.6.1 fixes the PHP warnings introduced in WPEC 3.8.7.6.

Tiny micro release: Let's pray to God that we'll never get to this.

In short, the major release is the only one where we can include new features in. For each major release, there should be a planning phase of what features do we introduce into that major release, what features should we spin off as a separate plugin, and what architectural aspects should we refactor. This can be called the spec.

When the spec is decided, we get down to code and testing. During this process, of course we can add to or remove things from the spec, but we should try to minimize that. When the features are all implemented, it's time to freeze it.

Freeze means we're not allowing any new features to creep in any more. As soon as a release goes into frozen state, the feature set is firmly decided, the major bugs have been fixed. From then on, we should strictly only commit patches for new bugs introduced by the release itself (called regressions). The point is to ensure that the release gets more and more stable by not introducing big changes that could potentially lead to new bugs.

This explains why every time we freeze for a beta release, we need to create a separate SVN branch. The purpose is to decouple the frozen state from the "bleeding-edge" state. For example, as soon as 3.8.8-beta is announced, I created a separate branch for 3.8.8 where from that point on, only bug fixes should be committed to that branch, where as the original branch-3.8 is now reserved for bleeding edge development (3.8.9). All bug fixes that are committed to frozen branches (3.8.8, 3.8.7) should also be committed to bleeding edge branches (3.8, trunk).

Commit Privilege, Code Review and Quality Control

Currently, the following members are actively contributing to WPEC (not in any particular order):

- Dan Milward

- Michelle Allen

- Justin Sainton

- Ben Huson

- Lee Willis

- James Collins

- Cameron Grant

- Jeff Ghazally

Let me know if I left out any names. These are the members with commit privileges.

Our previous release (3.8.7.6) shows that we need to enforce a stricter code review and quality control. As a result, I would like to propose the following rule:

You can only commit a patch if:

• There's already a ticket for that particular issue on Google Code. If there is not any, create one first, and upload your patch there. Don't jump the gun and commit it right away.
• The patch that is being considered for committing must be approved by at least 3 of the core committers. By "approve" I mean it must have been reviewed for code quality, as well as tested thoroughly by the core committers. You can't just say "yeah it looks good", but you have to test it yourself.
• The ticket is marked with the label Workflow-Commit.

There can be exception to this rule. If we're developing a new feature that consists of multiple small commits (like  the work I'm doing with theme engine), here's the process:

• Create a ticket on Google Code
• Create a separate branch for your development. You can either use Google Code, or you can fork http://github.com/instinct/wp-e-commerce and work on your own fork.
• When you're done with the development, post a comment on your ticket, link to your branch, and ask for code review and testing.
• Again, at least 3 of the core committers should be consulted and approve of the changes you made.
• After you get the approval, now you can merge your branch to the main release branch.

I know this would complicate and potentially slow down development a bit, but it is extremely important that every single line of code is accounted for. We can't risk the embarrassment like 3.8.7.6.2 any more.

If you're not a core committer (i.e. not in the list of names above), you're very welcome to review the code, test and put your comments in the issue tickets.

One thing to note when reviewing code and approving patches: For the one reviewing code: don't be an asshole, but be very critical and constructive, and question everything. For the one requesting code review, don't take it personally and don't think of getting revenge on the person critiquing you. 

Communicating With Our Beloved Users

It's also important that we communicate our plans for each release to the public. After all we're doing all of these things for them. Their businesses depend on our software.

Each time we decide on a spec for a new release, we should publish it somewhere (preferably our blog), get some feedback and answer the users' concerns.

For every release, as Amanda suggested, we should only tag the release and announce at the beginning of the day, and preferably at the beginning of the week. The purpose is to always be available when someone needs help. After a release is announced, at least one member of the core team should be always present and reply to users during the first 12-hour. If there's an urgent issue, we need to put together a fix, send the reporters a zip file containing the fix, put the link on the announcement, and work on releasing the bug fix release the following day.

One more important thing. Instead of mainly communicating new releases via blog, we should start offering users the choice of receiving announcements via email.

Any suggestion?

Do you have any suggestion regarding this guideline? Any question or confusion? Anything else that we need to address? Feel free to comment!

[Gary Cao]

Sorry, just want to mention that I forgot to put my name in the list of core committers, otherwise people might think I'm running away from the responsibility :)

[Screaming Code Monkey]

Hi Gary 

Thank you so much for the deployment strategy for wpec I see you have put quite a Lot of effort into it, and making sure that it is well understood by everyone. 

My only thoughts are how a patch gets approved by the 3 core committers, I feel there should be a system in place for this,, personally I have ( due to lack of understanding the proper commit procedures git svn , oh my! And also due to other commitments ) been creating issues and adding patches which never get picked up , I've tried getting responses and people to review them but nothing and as time goes by and releases launched the patches tend to rot ( not sure what the proper term is ) so how can devs be sure their patches are picked up?

There are two patches on gc i know i have supplied that have gone un noticed 

1. Download max timeout - this is so old and the issue still has not been resolved 

2. Personal message not appearing on transaction results page or purchase report 

I have thought about committing it myself but did not want to do so as I too believe code review should be done prior to any commit, in saying that i think which branch and release launch should be devided during code reviews ? No?

I'd also like to know how and when a spec gets built will there be an open forum or is this something that you decide and pass on through the mailing list?

Thanks again and I'm all for your proposal!

Best

Jeff

Sent from my iPhone

--
You received this message because you are subscribed to the Google Groups "WordPress e-Commerce Plugin Development" group.
To view this discussion on the web visit https://groups.google.com/d/msg/wordpress-e-commerce-plugin/-/1RNco3b-NDcJ.
To post to this group, send email to wordpress-e-c...@googlegroups.com.
To unsubscribe from this group, send email to wordpress-e-commerc...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/wordpress-e-commerce-plugin?hl=en.

[benjaminhuson]

Thanks for clarifying the branch structure Gary. I think you're completely right in your suggestions for how to have a more robust workflow and commit structure.

If we know that that is the procedure I think it's up to core dev to now be more proactive about checking google code for patches.

There are 3 filtered. To help identify and flag issues:

Workflow-needsreview (no patch yet)

Workflow-haspatch

Workflow-needstesting (has patch)

Workflow-commit

When something's flagged as has patch or needs testing, this is when it needs reviewing by core team.

If you're the 3rd core member to review I guess change to commit?

Ben

[Gary Cao]

Hi Jeff,

Thanks for the comment.

I have to admit that "patches which never got reviewed" is one of the problems that we have to commit to solve. I remember those two tickets that you mentioned. I haven't had a chance to properly test those two patches. Please give them a Milestone-3.8.9 and we'll definitely review them together this time, for sure.

I guess after 3.8.8 is officially released, let's return to IRC (the old channel for wpec-dev) and go over each ticket marked with Milestone-3.8.9. We should so a weekly bug scrub like WordPress core does, and actively test the patches. Jeff, have you figured out the privilege things in IRC? If you could put me as an admin of that channel, things will be easier for me.

I think we can put some more structure to how we approach the issue tickets. A weekly chat as before is good (but the time difference is a bit troublesome). We could also set up a site running P2 theme (like wpdevel.wordpress.com) so we can discuss things without simultaneously be there. The most important point is not to let tickets with patches gather dust.

Best,

Gary.

[Gary Cao]

Hi Ben,

Thanks for commenting. Yes, it would make sense that the 3rd core member to review will mark the issue as commit.

I think we can remove Workflow-needstesting, since a ticket with patch obviously needs to be reviewed.

If the patch is submitted by a core dev member, we can assume that the core dev has tested his / her own patch, so that means only 2 more devs need to review it.

Even as a ticket is marked as commit, we can wait until the weekly meeting to go over each one of them once again, and push the commits then.

At the end of each week, I hope that we can release an unofficial weekly dev build so that users who want to have quick fixes rather than waiting for the next release can test the fixes right away.

[Michelle]

Hi Gary

I like the idea of having an unofficial weekly dev build - I think it
will get more people testing the code and if released weekly and
problems are found it shouldn't be to hard to go back over the commits
from the week to see what went wrong
+1 for that idea

Also the IRC chat would be a great way to discuss patches and bugs get
together a weekly dev build so im for that idea as well - Time zones
make it tricky but im sure we could find a time that suits everyone who
wants to be involved.

-Michelle

> --
> You received this message because you are subscribed to the Google
> Groups "WordPress e-Commerce Plugin Development" group.
> To view this discussion on the web visit

> https://groups.google.com/d/msg/wordpress-e-commerce-plugin/-/-4zfYELrkAcJ.

[Screaming Code Monkey]

I'm in Ben and lees time zone now so this could get real tricky haha I'm not the owner or the channel unfortunately so you may need to talk to Valentina's on getting the channel sorted'

Best
Jeff

Sent from my iPhone

[Ben Huson]

Yes, times could be fun.
I'm certainly better with evening GMT
Just got IRC working on my iPhone which'll be useful too.

Just quickly back on Google code, note you can add an Owner against a ticket.
I presume that if one of us is actively working on a ticket we should
put ourself in the Owner field so that other devs know it is being
worked on a) not to duplicate effort and b) to know who to speak to
about it?

- Ben

On 26 January 2012 07:07, Screaming Code Monkey

[Screaming Code Monkey]

Yea,, and if an issue is reported by a core dev I guess they put themselves down as the owner? Gary when you work on your local git beanch do you mark yourself as the owner on the gc issue?

Sent from my iPhone

[Ben Huson]

If I'm reporting an issue, I only put myself down as the owner if I am
going to work on it.

If it's something that's I'm not sure how to tackle, or a bug fix I
don't have time to look at, I don't put myself as the owner.

I thought Owner was more of an indication of who was actively working on what?

- Ben

On 26 January 2012 12:07, Screaming Code Monkey

[Screaming Code Monkey]

Yea agreed I was more meaning if the core dev reporter supplied patch ..

Sent from my iPhone

[Lee Willis]

Hi Gary, 

Just thought I'd reply with some thoughts since at least one of my patches was involved in 3.8.7.6.1.

Regarding branches - there seems to be too many to me, 3.8 and trunk seem redundant to me. In a standard environment I'd expect to see:

• Trunk (Representative of current stable release)
• Branch 1 (Development branch for next release)

It seems strange to be simultaneously trying to focus on 3 separate development streams (trunk, 3.8 and 3.8.8) and handle porting of fixes across them. Obviously if you guys are happy with that then that's cool, but it would be tremendously helpful if the creation/remit of these branches were clearly communicated so if people are committing, or developing, then know what to target.

I'm also concerned a bit about the concept of major vs. minor releases and how it's being applied to version numbers. Historically API was considered stable across the whole of an x.x series. E.g. 3.7 vs 3.8, but if I read your mail correctly you're now only guaranteeing API compatibility at the third level? E.g. API can/will change between say 3.8.7 and 3.8.8 and minor releases are now 4 digits long, not 3?

I'm not quite sure of the logic for this, but it does cause problems. For example, 3.8.8 changes the IDs/Classes on the shipping forms. While this is no doubt being done for cosmetic reasons (The new admin looks MUCH better!), it causes problems for any plugins using AJAX within that area (Yes, HTML IDs/CLASSes are APIs too in the jQuery world). Not only that, but because it's being done at a triple-point release, there's no easy way to check for it, that I know of?

The committing strategy all seem to make sense, but I'd echo Jeff/Ben's comments about patches going nowhere - It'd be really nice to see a concrete suggestion of how we're using Google code to make sure these get picked up. 

Cheers

Lee

--

You received this message because you are subscribed to the Google Groups "WordPress e-Commerce Plugin Development" group.

To view this discussion on the web visit https://groups.google.com/d/msg/wordpress-e-commerce-plugin/-/1RNco3b-NDcJ.

[Gary Cao]

Hi Lee,

I totally understand your confusion. 

The confusing branches situation will only last until after we release 4.0. From then on, we'll only focus on 2 actively developed branches:

• trunk: the upcoming major release
• branch-4.0: the upcoming minor release for 4.0

This is also the branching structure that WordPress core uses (e.g. trunk now is for 3.4 development, and there's a branch 3.3 for minor releases).

I realize that I made a mistake calling 3.8.8 a major release in the original post, so I apologize for that. Also, minor releases can contain new minor features or improvements, as long as they don't break API backward compatibility.

The issue with the 3.8.x branches is that, 3.8 was a big enough architecture change to actually be released as 4.0. The "3" at the beginning of the version number suggests that there's no break in terms of API when compared to 3.7 or 3.6, while actually, 3.8 is totally different from 3.7. So I guess that's where the confusion is.

From 4.0, it will be much cleaner:

major.minor.maintenance

So, 4.0 and 4.1 will be using basically the same architecture. 4.1 might add some new minor features and improvements, but shouldn't contain any API changes that could break backward-compatibility.

In short, from 4.0 we will follow the same structure and convention that WordPress core has in order to minimize confusion.

[Lee Willis]

Cool - cheers Gary - all sounds good. 

PS. 3.8.7.6.2 doesn't seem to be tagged on the Google repo - only on WordPress.org ...

--

You received this message because you are subscribed to the Google Groups "WordPress e-Commerce Plugin Development" group.

To view this discussion on the web visit https://groups.google.com/d/msg/wordpress-e-commerce-plugin/-/F96RMoex7RoJ.