Sorry, of course feature should be merged to devel, not to master.

…
5)      If review is done fast enough to not allow devel to go further than 
root commit of cool_feature branch (which is the case almost every time), 
reviewer types “git checkout devel; git merge –no-ff cool_feature” and merges 
cool_feature on one hand on top of devel (which destroys probability of broken 
merge).
…

With best regards,
Viacheslav Ostroukh

Instituut Lorentz – Niels Bohrweg 2 – Room 259 – 2333 CA Leiden

ostro...@ilorentz.org
sl...@ostroukh.me

+31 6 444 968 12
+38 099 721 76 06



From: Viacheslav Ostroukh
Sent: Tuesday, October 20, 2015 13:57
To: Anton Akhmerov;Christoph Groth
Cc: kwant-devel@kwant-project.org
Subject: RE: [Kwant-devel] Thoughts about development workflow


Hi all,

So, there are a lot of talks about workflow, I will try to add my couple of 
propositions.

First of all, I don't feel that we need to organize huge automated build system 
site, self-hosted repo, unless we at least pay some system administrator. I 
think that Github-based workflow would be better because of iron stability and 
huge userbase. If we want to support our own "apt-get update && apt-get 
upgrade"-kind of infrastructure, I don't mind, if that guy, who supports it and 
catches all the bugs is not me, I am too married for it.

So, talking about workflow. My oppinion is Gitlab is better decision that 
Phabricator. I haven't used Phabricator, but seems it is oriented for slightly 
larger projects, than Kwant, because all our main developers can kick each 
other in Skype. KISS. 

About workflow. I propose following. Yes, idea of keeping Github mirror is so 
good, that almost mandatory. My proposal is to keep following branches:

-          Master – stable version, recommended for usage for beginners and 
lazy people “just give me a calculator”
-          Devel – semi-stable, should be rolled on infrastructure of Grenoble 
and Holland groups to catch bugs before master (if any). Beta builds are here.
-          Feature branches. All development is here.

Master and devel branches of development must be synced between Gitlab and 
Github. Feature branches can be not synced, however, may, if someone need merge 
changes from one feature branch in Gitlab to another at Github (and vice versa).

Ways to contribute:
-          Gitlab/Github – fork/pull request way.
-          For us: development in main repository in feature branches. (With 
all merges/rebases, if needed).
-          Sending a patch by email (old-fashioned, but still effective way).

Every change in devel branch should be reviewed. My proposition is that we have 
a number of reviewers (i.e. Joe, Christoph, Anton, Michael), that can merge 
into devel branch. If they are maintainers of feature branch, they should ask 
someone else to review. Master is just a well-tested snapshots of devel.

Proposed algorithm of workflow.
1)      Code gets to a reviewer and forms into feature branch cool_feature 
(with author or maintainer, if we got patch by email, for example).
2)      cool_feature branch gets tested (better to create automated tests or 
careful manual testing).
3)      In cool_feature branch “git rebase devel” is made. In best case 
everything is just put on top of devel, and cool_feature root commit is now 
HEAD of devel. If not, cool_feature maintainer fixes all merge errors and goes 
to (2).
4)      When feature branch is well-tested and put on top of devel, reviewer 
comes and checks everything. However, if devel branch runs forward, after 
reviewer check we need to go to (3) again.
5)      If review is done fast enough to not allow devel to go further than 
root commit of cool_feature branch (which is the case almost every time), 
reviewer types “git checkout master; git merge –no-ff cool_feature” and merges 
cool_feature on one hand on top of devel (which destroys probability of broken 
merge).
6)      Immediately reviewer syncs Gitlab and Github devel (no matter where did 
merging cool_feature performed.
Finally, we get nice not-complex commit history with almost zero probability to 
break devel branch. All features are displayed with clear and exact messages of 
type “Merged branch cool_feature into devel”.

This is my view of workflow. It worked well in small team earlier (with a note 
of no need to sync two Git mirrors). I hope it is good start for official 
contribution guide for Kwant.

With best regards,
Viacheslav Ostroukh

Instituut Lorentz – Niels Bohrweg 2 – Room 259 – 2333 CA Leiden

ostro...@ilorentz.org
sl...@ostroukh.me

+31 6 444 968 12
+38 099 721 76 06



From: Anton Akhmerov
Sent: Tuesday, October 20, 2015 12:33
To: Christoph Groth
Cc: kwant-devel@kwant-project.org
Subject: Re: [Kwant-devel] Thoughts about development workflow


Hi again,

Christoph Groth wrote:

> I didn’t say that everything should be squashed to one commit.  I just said
> that I think that the policy “explicit merges always” is not a good one.  I
> think it’s preferable to rebase a smallish isolated commit instead of
> merging it (this creates two commits and a more complex history graph).
> Gitlab actually has a feature to try to rebase a merge request and to apply
> it to the top if it rebases cleanly, but only in the “enterprise version”…

I agree, strict only-merge isn't crucial for minor fixes and clean
commits. The benefit of the Gitlab enterprise is mostly about GUI.

> For more complicated topic branches that consist of several commits it may
> be even clearer to merge them, as this groups the commits together and gives
> them a common name.  This is certainly true for feature branches with a
> convoluted history.  For feature branches that consist of a clean series of
> commits, I think it doesn’t matter so much whether they are merged or
> applied at the top.

I think this history http://antonakhmerov.org/misc/first-parent.png
can even be easier to navigate than Kwant history because the thematic
grouping of commits is explicit.

> The ability to participate in discussions by replying to email messages is
> already very nice.  Is it possible to make the emails sent by gitlab pure
> text (no HTML)?

It seems that all emails are controlled by templates:
https://gitlab.com/gitlab-org/gitlab-ce/tree/master/app/views/notify.
Probably erasing the html template one can make it pure plain text.
But it's a multipart message with plaintext seeming having all the
information, so I'm not sure whether it's important.

> Ideally, I think, it would be great if all the discussions could be mirrored
> to a mailing list (that could be a dedicated list, or, if the volume is
> acceptable, kwant-devel).  This would provide a convenient way to watch for
> new things without having to poll the website regularly.

* Having a gitlab account one can also sign up to all notifications
directly without a mailing list.
* It's also quite easy to sign up the mailing list to a custom subset
of notifications.
* Finally there are activity feeds as well.

> Well, I think even if we setup a gitlab instance, there will be still a
> barrier for people who live exclusively on github.  So it may be a good idea
> to have a github mirror of Kwant as well.  If we disable issues, we do not
> force anyone to use github, so if there is some volunteer to look out for
> PRs on github, that might be useful.

Sure, why not.

> While I like its more proper support for iterative code review, phabricator
> seems to be quite intrusive in various ways (It insists on adding cruft to
> all commit messages, for example).

I agree.

> If I undertand corretly, Gitlab, on the other hand, can be used as a mere
> git respository hosting server (like gitolite), so it does not force us to
> use any of its extra features.

It indeed supports a minimal mode of operation indeed, but we may as
well use several of its features: issue tracker, CI, and merge request
review.

Best,
Anton


On Tue, Oct 20, 2015 at 6:52 PM, Christoph Groth <christoph.gr...@cea.fr> wrote:
> Joseph wrote:
>
>> Christoph wrote:
>
>
>>> • (a3) Unnecessary merges are avoided.  Some people like a policy where
>>> each set of patches is merged explicitly.  I think this is bogus.  If a
>>> change can be put as a single commit (as it is the case very often), it
>>> should be best applied as a single commit on top of the current history.
>>> Unnecessary merges obscure the project history and make relations between
>>> features less evident.  This makes understanding the history much more
>>> complicated, and things like bi-secting for a bug work less well.
>>
>>
>> I would like to contest the point that a set of commits should be squashed
>> into a single one.
>
>
> I didn’t say that everything should be squashed to one commit.  I just said
> that I think that the policy “explicit merges always” is not a good one.  I
> think it’s preferable to rebase a smallish isolated commit instead of
> merging it (this creates two commits and a more complex history graph).
> Gitlab actually has a feature to try to rebase a merge request and to apply
> it to the top if it rebases cleanly, but only in the “enterprise version”…
>
> For more complicated topic branches that consist of several commits it may
> be even clearer to merge them, as this groups the commits together and gives
> them a common name.  This is certainly true for feature branches with a
> convoluted history.  For feature branches that consist of a clean series of
> commits, I think it doesn’t matter so much whether they are merged or
> applied at the top.
>
>> While this is a very admirable goal, I think that what is more important
>> is having a standard workflow that everyone can use. This lowers the barrier
>> to entry to new contributors. Currently the "you can contribute however you
>> like!" strategy is very nice if somebody already knows how to contribute to
>> an open-source project, but for a newcomer it is impossible to get off the
>> ground without some standard way of doing things. Of course the Standard Way
>> should probably also be the way the maintainers typically work, as they will
>> be the ones doing the review + making the most contributions.
>
>
> The ability to participate in discussions by replying to email messages is
> already very nice.  Is it possible to make the emails sent by gitlab pure
> text (no HTML)?
>
> Ideally, I think, it would be great if all the discussions could be mirrored
> to a mailing list (that could be a dedicated list, or, if the volume is
> acceptable, kwant-devel).  This would provide a convenient way to watch for
> new things without having to poll the website regularly.
>
>> The above improvements to the workflow can all be provided by either of
>> the tools proposed, however it is useful to have them preserved for
>> posterity here on the mailing list.
>
>
> Well, I think even if we setup a gitlab instance, there will be still a
> barrier for people who live exclusively on github.  So it may be a good idea
> to have a github mirror of Kwant as well.  If we disable issues, we do not
> force anyone to use github, so if there is some volunteer to look out for
> PRs on github, that might be useful.
>
>>> Before we continue with evaluating various tools, let’s first discuss
>>> what we would like to have.  I’m looking forward to your comments.
>>
>>
>> Essentially the features that you raised above. I think a transparent way
>> to do code review is important. Neither Phabricator nor Gitlab is perfect in
>> this respect. Phabricator only stores the diffs, not the commits that led to
>> a diff. This means that if a maintainer merges some accepted changes then
>> these are necessarily in one big commit. This can be annoying if someone has
>> implemented some feature with nontrivial changes where it makes sense to
>> keep the commits separate. In Gitlab the problem is that there is no history
>> of a patchset kept when dealing with merge requests[1] (although I have an
>> idea as to how this could be fixed).
>
>
> While I like its more proper support for iterative code review, phabricator
> seems to be quite intrusive in various ways (It insists on adding cruft to
> all commit messages, for example).
>
> If I undertand corretly, Gitlab, on the other hand, can be used as a mere
> git respository hosting server (like gitolite), so it does not force us to
> use any of its extra features.
>
> Christoph




Reply via email to