[PATCH] Re: Implement SSL certificate checking
Sebastian at SSpaeth.de
Wed Dec 15 02:22:10 GMT 2010
On Tue, 14 Dec 2010 21:56:09 +0100, Johannes Stezenbach <js at sig21.net> wrote:
> You're quick, that's good. But it's also bad...
> If you mix cleanups and functional changes, the patch is much harder
> to review. Please take the time to seperate the changes before asking
> others to spend time reviewing it.
I needed to "mix up" a few things, as introducing the cert verification
e.g. also reqiured getting rid of the way we did our ssl wrapping.
It's not like I posted a monster-patch that is totally convoluted, after
all it is "only" 3 files, 132 insertions(+), 72 deletions(-). I did
start work to break out some of the changes in a separate patch. I would
still like to hear whether the approach is ok, before I invest more time
in it. Release early, Release often is what I got told :).
> "Works for me" is inadequate. Seemingly working, but subtly broken
> crypto is much worse than no crypto. People need to be able to
> rely on it to protect their mail.
I do believe that the patch does the right thing. "Works for me" is a
necessary but not sufficient condition that the patch might be
acceptable. I never claimed that "Works for me" is a proof for the
correctness of the patch.
> I hope that doesn't sound too negative, I just think
> it's important to get that right.
So do I. That's why I post it for review :).
More information about the OfflineIMAP-project