On Sun, Feb 12, 2012 at 7:40 AM, Rob Lanphier robla@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