← Back to team overview

launchpad-reviewers team mailing list archive

Re: [Merge] lp:~mbp/launchpad/dkim into lp:launchpad

 

Hey Martin,

I like the refactoring that extracts extractAndAuthenticateCommands.

As a general point, we insist on capitalized sentences with full stops in our comments and docstrings.

I'm finding it hard to grok test_NonGPGNewBug. Your comment there is helpful, but I think it would help me trust the test more if there were reasons for why you aren't doing the normal thing and why you are logging in directly instead.

Also, you'll need to change all of the first person references. Nothing more frustrating to wonder who "I" is.

I also don't know how to make a GPG signature with crazy timestamps. It would be nice to do that instead of checking that the checker is called. The test you've got is OK to land though.

jml
-- 
https://code.launchpad.net/~mbp/launchpad/dkim/+merge/35985
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~mbp/launchpad/dkim into lp:launchpad.



References