[PATCH] 41 Changed the password input in pkidestroy from command line to a file. The parameter is still optional. Trac ticket #502
Please review the attached patch which adds a -W <security domain password file> option replacing the -w <security domain password file> option in pkidestroy. --Abhishek
On 3/22/2013 10:16 AM, Abhishek Koneru wrote:
Please review the attached patch which adds a -W <security domain password file> option replacing the -w <security domain password file> option in pkidestroy.
Some comments: 1. Please add a short subject line in the comment like the other patches. 2. Let's use the US spelling 'canceled' as explained in the other patch. 3. Could you find out what the strip('[\']') is used for? In any case, the original code strips those characters from the password. The new code strips those characters from the file name, not the password. 4. Please use 'with' to open the file. See: https://fedorahosted.org/pki/ticket/560 -- Endi S. Dewata
Please review the attached patch. On Fri, 2013-03-22 at 10:32 -0500, Endi Sukma Dewata wrote:
On 3/22/2013 10:16 AM, Abhishek Koneru wrote:
Please review the attached patch which adds a -W <security domain password file> option replacing the -w <security domain password file> option in pkidestroy.
Some comments:
1. Please add a short subject line in the comment like the other patches. Modified the subject line.
2. Let's use the US spelling 'canceled' as explained in the other patch.
Done.
3. Could you find out what the strip('[\']') is used for? In any case, the original code strips those characters from the password. The new code strips those characters from the file name, not the password.
Not required when password is input using a file.
4. Please use 'with' to open the file. See:
Used the with construct for file reading. --Abhishek
On 3/25/2013 3:10 PM, Abhishek Koneru wrote:
Please review the attached patch.
A few more things: The -u and -W options should be added into the SYNOPSIS section. The patch also needs a rebase. The trailing whitespace should be removed. The 'Conflicts' log message should be removed as well. Other than that it's ACKed. Please fix before push. -- Endi S. Dewata
Fixed the comments, pushed to master. --Abhishek On Tue, 2013-03-26 at 13:53 -0500, Endi Sukma Dewata wrote:
On 3/25/2013 3:10 PM, Abhishek Koneru wrote:
Please review the attached patch.
A few more things: The -u and -W options should be added into the SYNOPSIS section. The patch also needs a rebase. The trailing whitespace should be removed. The 'Conflicts' log message should be removed as well.
Other than that it's ACKed. Please fix before push.
participants (2)
-
Abhishek Koneru -
Endi Sukma Dewata