1. aragost Trifork
  2. Mercurial Extensions
  3. commitsigs
  4. Pull requests

Pull requests

#1 Declined

Support for SSH keys

  1. Jonathan French

I've added support for SSH keys, using Paramiko, to commitsigs. I needed it for signing integrated with mercurial-server; hope it will be useful for others as well.

There are a couple of fixes in there as well; for some reason, probably a bug in Mercurial, the config dictionary was being reset sometimes, so I made it a function and passed around the ui object instead.

(update: Oops, made a mistake in the docstring. Corrected.)

Comments (2)

  1. Martin Geisler


    That looks like interesting functionality. However, I have some comments:

    1. You don't have a real username in the changesets, just ojno.
    2. There are some really long lines in the new code -- please keep them below 75 characters to make the code easy to read.
    3. The dependency on Paramiko should be optional. So you will need to import it, access a function or class in the module and catch the ImportError exception thrown if the demandimport system in Mercurial cannot find Paramiko.
    4. The changes to the config system look good, but they should be done in separate changesets. That makes it easier for me to review: I can then pull in the good stuff first and we can talk about the more complicated things later.
    5. The "hi" here looks like temporary test code: 'ssh.publickeydirs': ["hi"].
    6. The new tests (great that you add tests!) should go into a new test-commitsigs-ssh file. But I can see that you just followed the pattern I laid out originally.

    Could you rework the changesets with something like MQ and submit another pull request?