[Pki-devel] [PATCH] 40-2 Comments on patch 40 addressed
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
Attachments:
- pki-akoneru-0040-2-Handle-the-Keyboard-interrupt-gracefully.patch (text/x-patch — 2.4 KB)