487793 - Add an hg hook to all l10n repos to remind about reviews being required for changes to the productization files

Status: RESOLVED WONTFIX

(Mozilla Localizations :: Infrastructure, defect)

Description

[Staś Małolepszy :stas]

Putting this in Mozilla Localizations > Infrastructure to discuss internally, first. 

I'd like to suggest that we add an hg hook to all l10n repos that would display a message to the committer if at least one of the changed files is region.properties or searchplugins/*. The hook would allow to see what changes will be pushed (in case they were involuntary, or result of a tool export) and would offer a chance to abort the push. 

What do you think?

I'll attach a proof-of-concept code in a minute.

Comment 1

[Staś Małolepszy :stas]

Attachment: The hook

Here's the proposed hook. 

To enable it, we'd have to put this:

[hooks]
pretxnchangegroup.productization_reminder = /path/to/productization_reminder_hook.sh

in every locale's central and 1.9.1 repo (in .hg/hgrc).

Comment 2

[Axel Hecht [:Pike]]

We should make sure this cooperates with GUI tools.

We should at least test against TortoiseHG, Eclipse and KomodoIDE.

Comment 3

[Zibi Braniecki [:zbraniecki][:gandalf]]

I'm worried about:
a) Us breaking the common paths of HG commits/pushing (which is complex by itself) by introducing the concept of user interactions on push
b) Us not solving the real case (no way to modify comment from here, no way to remove those files from the commit from here)
c) Us having to hack GUI tools we'll want to suggest to our users due to a)

Comment 4

[Staś Małolepszy :stas]

(In reply to comment #3)
> I'm worried about:
> a) Us breaking the common paths of HG commits/pushing (which is complex by
> itself) by introducing the concept of user interactions on push

That's a reasonable concern. I tested Eclipse with the Mercurial plugin, and it seems that the plugin can't handle hooks that require user input. Which basically means that it waits for the input, but doesn't display any kind of console. And it becomes unresponsive (you get the spinning beachball of death on mac) :/

> b) Us not solving the real case (no way to modify comment from here, no way to
> remove those files from the commit from here)

The idea is to prevent the push and make the localizers commit a backout changeset, and then push again. To prevent *commits*, we'd have to make them install the hook locally, because it doesn't get cloned with the rest of the repository with "hg clone".

> c) Us having to hack GUI tools we'll want to suggest to our users due to a)

We could evangelize text editors' developers to support this kind of behavior. Or we could change the hook to only notify the user that "changes to productizations files have been pushed, please make sure they're reviewed, if not, contact l10n-drivers or back them out." I tested this approach in Eclipse and it worked, although the message is sent to the console, which is hidden by default, so it doesn't really help, IMO..

Thoughts?

Comment 5

[Cédric Corazza]

I agree with Axel, please make sure this cooperates at least with TortoiseHG (Eclipse and OpenKomodo are used by tech savvy localizers imo, whom should represent a very small part of us).
Do you have figures btw: I didn't see anything about the Mercurial tools used on m.d.l10n?
And don't forget the ones that use hg command lines ;-)

Comment 6

[Axel Hecht [:Pike]]

I'm going to resolve this WONTFIX. It seems that we're over-engineering the problem here, and also, we haven't raised this in the past three years, so cleaning this up is the right thing to do.