Github cleanup

[Paul Backhouse]

Hi all,

We have 6 merged branches on github:

users_can_create_orgs, editor_role, comment_notifications, stylechange,
paulb

Can I remove these now? It seems like these were probably all things I
worked on, but it doesn't hurt to ask.

We also have several unmerged branches:

develop, action_items_2, dye, action_items, recaptcha_registration,
marko

Which of these are under active development, or contain useful code?

I wrote "recaptcha_registration", which contains a few lines of code
that may be useful at some point, but nothing that you couldn't figure
out from the apps doc site.

Paul

[Sarah Bird]

Sounds good to me.

action_items_2 IS under active development (do not delete)

action_items IS not (please delete)

(I am about to start an action_items_3 also)

Cheers,

Bird

Paul

--

--
skype: birdsarah
email: sb...@alum.mit.edu
web: www.sarahbird.org | www.bonvaya.com

[Hamish Downer]

> We also have several unmerged branches:
>
> develop, action_items_2, dye, action_items, recaptcha_registration,
> marko

dye is a branch I created to try putting in the new version of the
deployment scripts I have been working on:

https://github.com/aptivate/dye

It's basically ready to go, so I've been hoping to find some time to
check in with everyone, merge it in and write an email about what it
changes, but I haven't got there yet.

To reassure you, I am using the dye version of the scripts in INASP and
they've been working fine.

Hamish

[Paul Backhouse]

In Fri, 2013-01-11 at 12:19 +0000, Hamish Downer wrote:
> dye is a branch I created to try putting in the new version of the
> deployment scripts I have been working on:
>
> https://github.com/aptivate/dye

I've left it in and created a merge task in kanban.

[Jo Paulger]

I'm still working on the basis that all development branches are to be merged into the 'develop' branch, which is then released to the staging hub for QA testing, so unless this workflow changed recently, please don't delete the develop branch.

Jo

--

[Hamish Downer]

> dye is a branch I created to try putting in the new version of the
> deployment scripts I have been working on:
>
> https://github.com/aptivate/dye
>
> It's basically ready to go, so I've been hoping to find some time to
> check in with everyone, merge it in and write an email about what it
> changes, but I haven't got there yet.

The major change for now is that the custom stuff in fabfile.py is now
split between project_settings.py and localfab.py - while fabfile.py
becomes a totally standard file (like tasks.py/tasklib.py/localtasks.py)

I have a slightly more ambitious stage 2, where dye will be checked out
into a virtualenv, and then I won't have to copy all the files to and
from an upstream project so much. I've written most of the code for
that, but need to try it out when I have some time. But the new dye
files can work in both modes.

Hamish

[Sarah Bird]

Agreed, to refresh the process:

pull requests go against develop, and the card goes into "code review" column
After code review, code is merged into develop and cards go into "QA testing" column.
After testing, cards go into "Merge us please" column waiting to be merged into master.

Given that there are no cards out in QA Testing or Merge us please, develop should currently be in-sync with master.

We should have a look at this on Monday before we start code reviewing.

Cheers,

Bird

--
 
 

[Paul Backhouse]

> Given that there are no cards out in QA Testing or Merge us please,
> develop should currently be in-sync with master.

My bad. I've done a few pushes recently to develop.

The good new is that develop has not been deleted.

So sorry!

Is it OK to push to develop for hotfixes?

Paul

[Sarah Bird]

Hi,

yeah, I spotted that during the long code review process email I sent.

I would say, it depends.

1) Do you want those fixes to be code reviewed, if yes, why not leave them as pull requests until they are?
2) Do we want to code review all commits. I'm not sure we do, but I'm not sure we've been explicit about the policy.

Cheers,

Bird

Paul

--

[Paul Backhouse]

On Mon, 2013-01-14 at 03:12 -0800, Sarah Bird wrote:
> 1) Do you want those fixes to be code reviewed, if yes, why not leave
> them as pull requests until they are?
> 2) Do we want to code review all commits. I'm not sure we do, but I'm
> not sure we've been explicit about the policy.

I suppose the reason I'm pushing to 'develop' is that a lot of the tasks
we're working on right now are bug fixes. It doesn't seem right to
create a feature branch and a pull request for each fix when the fix
itself can be one,two or three lines of code.

So I'm still having trouble, in my head, marrying up individual cards to
specific github activity that doesn't result in weighty bureaucracy.

How about we each have our own 'hotfixes' branch, in our own github
repo, and we can put all bug fixes there?

Does that work? We could even do without such a branch and do bug fixes
(or other small work) directly on our own 'develop' branch, and then
pull request that back to origin/develop.

So we still maintain the code review process, but lots of fixes will be
in one pull request.

Thoughts please.

Paul

[Sarah Bird]

Hi,

Yeah, I'm definitely not advocating for a feature branch for each bugfix. Some options:
- have you're own fork of develop on github, and submit commits to that as pull requests to main develop
- have your own branch paulb, and submit commits to that as pull requests

However, I don't see the benefit of "lots of fixes will be in one pull request."
- it's one click to make a pull request, so very little effort needed
- one kanban card / bug per pull request keeps it much clearer what the commit does, so its very easy to do a code review.
- Github then makes it easy to merge just that one thing

Cheers,

Bird

Paul

--

[Jo Paulger]

I'm afraid I don't know how to submit a pull request on one commit (or on a set of commits) - as far as I understand it, you have to submit a pull request on a branch, so how can we keep to one kanban card per pull request without creating a specific branch for each kanban card? 

Within my feedback_fixes branch (on my github only - not shared yet), I have covered several kanban cards, grouping them to save copious branching and because they are all small screen issues with feedbacks, but have tried to keep it to one commit per kanban card, because I reckoned that would help during code review (you can choose to review a pull request's changes all folded together or on a commit by commit basis). If there is a way of submitting one commit as a pull request then I'll happily split them up. I'd like to point out that another reason for grouping similar fixes on one branch is that sometimes it's really hard to test your fix for one bug until another bug has been fixed, and I've learnt the hard way that it's not a good idea to merge one of my personal github development branches into another before submitting them both as pull requests! Lately I've started to include the URL of the kanban card in the commit message for the fix, with the intention of easing the code review process.

How about we use this week's code review blast to compare and contrast the different approaches before coming to a conclusion about the best way to go forward? 

In my experience, the harder it is to do code reviews, the more likely it is to get neglected :(

--
 
 

[Paul Backhouse]

On Mon, 2013-01-14 at 15:11 +0000, Jo Paulger wrote:
> I'm afraid I don't know how to submit a pull request on one commit (or
> on a set of commits) - as far as I understand it, you have to submit a
> pull request on a branch, so how can we keep to one kanban card per
> pull request without creating a specific branch for each kanban card?

We can put a comment in kanban with the commit id, and then cherry pick
that commit if need be.

> How about we use this week's code review blast to compare and contrast
> the different approaches before coming to a conclusion about the best
> way to go forward?

Good idea.

[Sarah Bird]

Hi Jo,

Good news: it's easy to submit a pull request against a commit. Look at the commit in github and you'll see a pull request button at the top.

I'd be happy to chat with you about the different options you have whenever's a good time.

Totally agree about "it's really hard to test your fix for one bug until another bug has been fixed" and "I've learnt the hard way that it's not a good idea to merge one of my personal github development branches into another before submitting them both as pull requests!"

My suggestion was not about one size fits all - i.e. it's not about one pull request per commit or one pull request for an entire branch. It's about optimized pull requests to help reviewers. As you said. "In my experience, the harder it is to do code reviews, the more likely it is to get neglected :(" Code reviewing tends to take a lot of time, so we want to help each other and minimize confusion so we all get good feedback. So I was just trying to be constructive on that front.

Code review process as a work in progress +1 -> there's always time for retrospection and refactoring :D

Cheers,

Bird

On Mon, Jan 14, 2013 at 7:11 AM, Jo Paulger <joanna....@gmail.com> wrote:

rouping them to save copious branching and because they are all small screen issues with feedbacks, but have tried to keep it to one commit per kanban card, because I reckoned that would help during code review (you can choose to review a pull request's changes all folded together or on a commit by commit basis). If there is a way of submitting one commit as a pull request then I'll happily split them up. I'd like to point out that another reason for grouping similar fixes on one branch is that sometimes it's really hard to test your fix for one bug until another bug has been fixed, and I've learnt the hard way that it's not a good idea to merge one of my personal github development branches into another before submitting them both as pull requests! Lately I've started to include the URL of the kanban card in the commit message for the fix, with the intention of easing the code review process.

How about we use this week's code review blast to compare and contrast the different approaches before coming to a conclusion about the best way to go forward? 

In my experience, the harder it is to do code reviews, the more likely it is to get neglected :(

[Paul Backhouse]

On Mon, 2013-01-14 at 07:45 -0800, Sarah Bird wrote:
> Good news: it's easy to submit a pull request against a commit. Look
> at the commit in github and you'll see a pull request button at the
> top.

Hmmm. Clicking on "pull request" while viewing a commit takes me a page
that creates a pull request for the whole branch.

[Sarah Bird]

Hi,

I don't know what I've been smoking, but I completely agree with you. I could have sworn you can pull request against a commit, but I can't see any way of doing it - you're right.

My sincere apologies, this has definitely guided my thinking in the wrong direction. I'm really sorry for the curfuffle.

This definitely changes my opinion about pushing directly to develop. I would agree its too much to create a branch for a bug fix.

Jo, go ahead with what you were going to do.

Really sorry.

Bird

--