[Pki-devel] [PATCH] 40-2 Comments on patch 40 addressed

Abhishek Koneru akoneru at redhat.com
Mon Mar 25 20:04:28 UTC 2013


Please review the attached patch.

On Fri, 2013-03-22 at 10:15 -0500, Endi Sukma Dewata wrote:
> On 3/20/2013 5:08 PM, Abhishek Koneru wrote:
> > Please review the patch with fixes for ticket 536 - Catch the keyboard
> > interrupt during the execution of pkispawn and pkidestroy.
> 
> Some comments:
> 
> 1. Please add a short subject line in the comment like the other patches.

Comment modified to have a short subject line
> 
> 2. Any reason you use signal handler instead of try-except?
> 
> 3. If we keep the signal handler, the signal.signal() invocation should 
> be moved into the main program so it's easier to read.

Decided to go with signal handler rather than KeyboardInterrupt after
discussing with Endi. Moved the handler invocation to main program.
> 
> 4. Does it print a single blank line between the interrupted line and 
> the 'canceled' message? See the example:
> 
> https://fedorahosted.org/pki/ticket/536

Message follows the example in ticket.
> 
> 5. The US spelling is 'canceled' instead of 'cancelled':
> 
> http://grammarist.com/spelling/cancel/
> 
> I think in the future we'll provide a proper translation & spelling for 
> each locale, but to be consistent for now let's use US spelling as the 
> default.
> 

--Abhishek
-------------- next part --------------
A non-text attachment was scrubbed...
Name: pki-akoneru-0040-2-Handle-the-Keyboard-interrupt-gracefully.patch
Type: text/x-patch
Size: 2464 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/pki-devel/attachments/20130325/727c0646/attachment.bin>


More information about the Pki-devel mailing list