[PATCH 4 of 4] keyring: add new extension keyring

Augie Fackler lists at durin42.com
Mon Sep 17 09:28:04 CDT 2012

On Mon, Sep 17, 2012 at 9:13 AM, Markus Zapke-Gründemann
<markuszapke at gmx.net> wrote:
> Marcin Kasperski schrieb:
>> Two remarks from my side (mercurial_keyring maintainer):
>> a) I am open to accepting coding standard patches to mercurial_keyring.
>>    Preferably in some chunks so diff is readable…
>> b) In my personal opinion the best sequence of things is
>>    1. Patch Mercurial core so password management can be plugged in
>>    without monkeypatching (and plugin gets some necessary info - which
>>    password, for which user (if set), was the password wrong…)
> So you think the hooks I added to the http modules are not enough?
>>    2. Patch mercurial_keyring (still kept separate) so it uses this
>>    possibility.
>>    3. Adapt mercurial_keyring to mercurial coding standards etc.
>>    4. Consider moving mercurial_keyring to mercurial core.
>> Point 1. may be of use not only for mercurial_keyring, there are many
>> password management schemes and tools nowadays…
> I would propose a different order for moving the extension to the core:
> 1. Add a test suite
> 2. Apply Mercurial's coding conventions
> 3. Remove old and unused code (needs to be tested w/ different Mercurial
> versions to be sure not to break anything)
> 4. Move mercurial_keyring to the core and remove the monkeypatch code
> I'm sure I could reuse the test suite I wrote for keyring because I started it
> originally with mercurial_keyring.

This is more or less what I did with histedit, and it worked out
reasonably well. I think in the end I had some cleanup changes on the
hg side of things even after the initial histedit cleanup.

I might refactor hg so that it can support mercurial_keyring without
the monkeypatch, then update keyring to use the new hook, *then* move
the extension into core. YMMV.

> Markus
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel

More information about the Mercurial-devel mailing list