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

Markus Zapke-Gründemann markuszapke at gmx.net
Mon Sep 17 09:13:23 CDT 2012


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.


Markus


More information about the Mercurial-devel mailing list