[Aptitude-devel] On working together (was: On reviews, supervision, etc)

Fri Feb 7 00:30:10 UTC 2014

2014-02-05 14:42 Axel Beckert:
>Hi,
>
>I must admit, I only have read the "tl;dr" part of the mail I'm
>replying to, but I'm sick of these heated discussions despite I think
>I can understand both sides. (I may read that later but I think I
>already have read enough mails in the past few weeks to have an idea
>where the different views clash.)
>
>I would like to de-escalate the situation and to come to some common
>ground so we are all able to get working on aptitude again.

Thanks for trying to get this sorted out, it's not a pleasant
position.

>The following workflow is based on the these ideas:
>
>* If someone wants to review changes by others, he should have a fair
>  chance to do that.
>
>* If somone thinks, he has a better solution, he should have a fair
>  chance to show that.

Probably this sounds defensive, but since it's mostly me who did the
actions in the last few weeks and who received the complaints by
Daniel for them, I am going to talk as if I am the main cause to raise
the points above.

I am not sure if you mention this in general (which is fine for me, so
no objections in this case), or implying that the discussions happened
in the last few weeks were in part because I did not allow reviews or
changing things that I did wrong, in which case I don't agree, so I
want to give the impression that I oppose this.  Because in fact, the
several times when there were points raised I fixed things and did it
quickly enough.

Summary of changes / timelines:

- The changes towards 0.6.3-1 were released fast, but crucially, they
   were all minor updates (translations), fixes to documentation,
   bumping versions of automake & friends, or fixes to code already
   present in the NMUs for months or the VCS, and created by Daniel.
   So I don't think that anybody had anything to say against them and
   that's why I didn't try to ask for reviews or wait.

   Besides that, the development was completely stalled (nobody around,
   apparently), and I had a free weekend to incorporate the changes.
   If I waited for a few weeks to get the green light probably resuming
   development would not have happened (which is a bigger loss than
   lack of reviews, IMO, even if this was the case).

- The changes towards 0.6.4-1 were in the same vein, mostly minor
   updates to translations and documentation, picking very
   straightforward patches, or things like the "debsize" policy already
   accepted years ago in branches of the repository by Daniel as well.
   I only added one feature by myself, "Add "Reinstall (shortcut L)" to
   the package menu (Closes: #633788)", which is also straightforward.

   Daniel reverted the change after the newline (meaning: he was
   already reviewing code; and that was one of the more recent changes,
   meaning: not lagging behind), and that was a couple of days before
   the release, with only a minor addition since then (descriptions for
   the "new" 3 sections).  So I didn't think that there were any
   objections.

- Since the last release, the changes were also pretty minor with only
   1 new feature (the new sorting policy of "installsizechange"), and I
   fixed the couple of points raised.

- But most importantly, in all those cases I advertised specially in
   the BTS to the submitters (and anybody who wants to chime in); in
   all cases the changes are very very minor and unlikely to cause any
   significant problem (they are not going to corrupt the package DB or
   anything like that), so there's no big problem even if they fail
   spectacularly (and they were often fixing segfaults, so it would not
   be worse).  And being released in Debian unstable/testing is not a
   big issue, the changes can be reworked easily in a new upload if
   needed.

Lastly, I'd like to say that I am intentionally holding off any change
other than small fixes and features (requested in BTS), so I am not
sure if the changes can be any less controversial unless I only fix
typos.

>* If someone has no time, this should not hinder others to continue
>  their work.
>
>* If someone works on long-term features, this should be visible.
>
>* Try to get some consesus instead of having someone playing "boss".
>
>* It should be able to do bug-fixes in a "release early, release
>  often" style.

OK to these.

>So I propose the following team workflow:
>
>* Minor and obvious bug-fixes intended for the next upload should go
>  directly to master or debian-sid (depending if they target upstream
>  or packaging code) without any review or similar. In case of doubt,
>  use a feature branch. (See below.)
>
>  All bug-fix-only releases should have urgency=medium or urgency=high
>  depending on how critical the contained bug fixes are.

I think that the paragraphs are a tad incompatible, because
minor/obvious fixes will very rarely be "urgency=high".

"urgency=medium" is the new default coming with the packaging tools
and blessed by Release Team apparently.  I don't think that there's
much difference between any of them in practical terms, specially not
between low and medium.

<defensive mode warning>

Personally, I think that all changes commited in the last weeks were
both minor (non-invasive like changing behaviour, or touching
important parts like the resolver) and obvious bug-fixes; except
perhaps the addition of the sort policies (because being wishlist and
not proper bugs, except in terms of consistency).  But "debsize" had
already been accepted 18 months ago, and the menu entry for reinstall
can do little harm.

So perhaps there must be more discussion and consensus on what's
minor, what's invasive, etc; I think that this is one of the cores of
the problem, we think that we talk about the same thing but we are
not.

Other than that, the common practice so far (as explained by Daniel in
emails long ago) was to commit them to master so far, that's why I
tried to continue doing this.  The only "different" branches are
wip-cmdline with many disruptive changes, and experiemntal.

</>

>* Features or invasive bug fixes should be developed in a feature
>  branch, preferably named <username>/<feature> or
>  <username>/debian-<feature> depending if they target upstream or
>  packaging code, e.g. "abe/german-doc" or "abe/debian-german-doc".
>
>  Unless stated otherwise by the owner of the branch, please do not
>  commit stuff to someone else's feature branch without an OK of the
>  owner. Use your own feature branch with the same feature name in
>  case of doubt.

I am fine to use this if it's going to avoid problems, although I
think that this is mostly an unnecessary (if minor) overhead for the
changes that we've been doing so far in the last few weeks.

Overhead mostly in the sense that one has to remind reviewing what's
pending or not after a week, instead of simply commiting to master and
forget if the are no objections.

For the time being, I don't plan to do any changes which represent
changing more than 100 lines (unless automatically generated, like
updates to .po files), nor work in flashy new things or big changes in
behaviour.  Not anything that cannot be accomplished in a few hours
(because I simply cannot aford bigger time spans, or things that need
multi-week efforts).

In any case, I suppose that I can use "mafm/master" or
"mafm/master-next" for example, and use that for all my little
fixes/features, as long as they don't clash?

>* As soon as its author thinks the feature is ready for inclusion, he
>  writes an e-mail to the according bug report or, in case of no
>  related bug-report, to the aptitude-devel list directly, including
>
>  + Either a link to a branch or a single commitdiff on
>    anonscm.debian.org, or with a single git formatted patch attached.
>
>  + A short description of what the patch or the commits in the branch
>    do. If the patch is attached, the included commit message should
>    suffice.
>
>  If there's no objection against the proposed patch within 6
>  days(*) or if there's a _consensus_ anyway, the branch may merged
>  into master, debian-sid or in one of the feature fe branches.
>
>  If you have objections, please provide an alternative patch in your
>  own feature branch and send that for review, too. If there's an
>  objection without alternative patch for more than 7 days(*), the
>  feature branch in question may be merged anyway.
>
>  For now I'll leave explicitly open what happens if someone thinks
>  that a feature should not included in aptitude by all means and the
>  author of the patch disagrees, or if we have two opposing
>  implementations whose authors object the other's patch -- because I
>  really hope that we are all sane enough to not let it come that far.
>
>* Finetuning of already merged branches should preferably be continued
>  in the feature branch which is then later merged again, probably
>  with a shortened review interval.
>
>* There may be seperate branches for new feature releases. They should
>  be handled like the master branch.
>
>* Branches should be pushed at least once a day if they received local
>  changes, so that others (also people outside the team) can see
>  what's happening in aptitude and can review it if they want.
>
>* If you commit code written by someone else, please properly
>  attribute it by using "git commit --author='Name <email>'" (if not
>  already given in a merge request or git formatted patch).
>
> [...]
>
>(*) Other timeframes are debatable, but I don't think anything less
>    than 2 days or more than 10 days makes sense. 6 days would allow
>    to merge code written on Sunday late evening to be merged on next
>    Sunday in the afternoon, hence without time pressure in the
>    evening and 7 days include one full week-end of time in any case.

Same as above, OK for me.

Feel free to point if I am doing this wrong though, I am a bit lost
with the terminology.

>* If you're not (yet) a team member, you're welcome to contribute
>  anyway and publish your feature branches elsewhere, either on Alioth
>  under the "user/" hierachy or on Gitorious, GitHub, etc. or on your
>  personal Git server. Be sure to include a working URL to anonymously
>  pull your changes in that case.

I am confused with the "user/" hiearchy, can you please explain?

>I hope this allows everyone to contribute without the need of heated
>discussions and avoids that the general development of aptitude stalls
>again despite there is someone who wants to continue working it.

Thanks again for your mediating efforts.

Cheers.
--
Manuel