On Sun, Feb 12, 2012 at 7:40 AM, Rob Lanphier <robla(a)wikimedia.org> wrote:
Which reminds
me, does LocalisationUpdate support git?
Not yet:
https://bugzilla.wikimedia.org/show_bug.cgi?id=34137
That bug is not about LocalisationUpdate. LU is not a TranslateWiki
tool, it's a WMF-side extension for pulling in up-to-date translations
(which happen to come from TWN, but it doesn't care about that). See
https://www.mediawiki.org/wiki/Extension:LocalisationUpdate for
documentation (details are slightly outdated, but the main points are
accurate).
LocalisationUpdate has SVN support built-in, but WMF stopped using
that a long time ago due to performance issues. Instead, the wrapper
script that we use to run the LU update on the cluster updates a local
checkout of the SVN repository, then passes the path as a command-line
parameter to the LU update script. This wrapper script is managed by
puppet, and is in the puppet git repository [1].
> I read through almost all of the commit mails. If
I need to spend any
> more time on it or do more clicking around my head will probably
> explode. This means that I want commit emails with links to review
> tool,
These exist. Gerrit sends notifications when an event occurs (creation
of a new change, comment/review on an existing change, new version of
an existing change, change merged), and it sends these notifications
to all users that have commented on the change, as well as the author.
I believe that Mark B gets e-mail notifications for all new changes in
the puppet repository (I think he said this once, not sure), so that
would seem to mean that setting up a mailing list that all
notifications go to is possible.
These notification e-mails have links to the change in Gerrit, and
also include a description of who did what (e.g. "Mark Bergsma
submitted this change and it was merged") and, for comment
notifications, the text of the comment.
> review tool should load fast, display all diffs
inline (not
> hidden behind links or anything)
That's a problem. I also want a full diff
(you can't get that with
Gerrit currently, only per-file diffs) that's displayed inline (can't
do this either, diffs are displayed in new tabs). The only workaround
that exists is that clicking the (kind of hidden) "Diff All Unified"
button will load all of the per-file diffs at once, i.e. it will open
a new tab for each file in the diff. I find this sort of bearable, but
it's really not nce.
This is the first thing I tried to fix once I got my hands on a
working dev install of Gerrit, but since I don't have that much Java
experience, I didn't really get anywhere.
> and I should be able to filter
> commits by path or author.
Mark does post-commit (after the fact) review of
puppet changes
sometimes, and he's sort of indicated that that doesn't work very
nicely; Mark, could you pitch in here and tell us what works for that
and what's lacking?
Per-author filtering of commits is implemented for sure, and you can
also filter for unmerged commits by a given author. There are no path
filtering features that I know of, at least for unmerged commits. I'm
pretty sure Gitweb can do path filtering for commits that have already
been merged into the mainline.
> In addition I want to be able to tag
> commits (for l10n team to review or keeping list of commits I want to
> deploy to the live site).
There is some notion of a "topic", which is
a kind of tag that the
committer applies when they submit their commit into Gerrit. The topic
is usually the name of the branch the committer used locally, or a bug
number, and it's used to group related changes. But you can't change
the topic after submission AFAIK, and you can't have multiple topics
for one revision. So it's not real tagging as it exists in CodeReview.
Any failure to
meet these requirements is
highly likely to be a blocker for me. Oh and something that signs that
this commit has followups that fixes problems in it.
Follow-up marking is
also lacking, as far as I know. The way I've been
doing it is to include the change number or Gerrit URL of the
followed-up revision in the commit summary, but that doesn't list the
follow-up in the reverse direction (i.e. if I commit B as a follow-up
to A, B will say it's a follow-up to A, but A will not say it was
followed up on in B). It's true that follow-ups should be less common
because commits can be amended while they're being reviewed (so there
will be no follow-up commits for things that are caught during
review), but of course reviewers aren't perfect, so if something
passes review (and unit tests!) but is still broken in some way, it'll
still need a separate follow-up commit.
I copied the issues you raised to the "Gerrit
bugs that matter" page.
I also added a tracking bug to make sure we triage that list:
https://bugzilla.wikimedia.org/show_bug.cgi?id=34349
I've edited that page a bit to add the notes I added above.
Specifically, having notification e-mails is not a "Gerrit bug that
matters", it's a feature that has already been implemented. Please
only add things to the bugs list after you've verified they're not
actually already in Gerrit :)
Roan