[PATCH 4 of 4] keyring: add new extension keyring
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
> 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.
More information about the Mercurial-devel