[PATCH] 41 Changed the password input in pkidestroy from command line to a file. The parameter is still optional. Trac ticket #502
10:16 a.m.
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
10:32 a.m.
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
3:10 p.m.
New subject: [PATCH] 41 -2 Fixes for review comments for Patch 41
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:
https://fedorahosted.org/pki/ticket/560
Used the with construct for file reading.
--Abhishek
1:53 p.m.
New subject: [PATCH] 41 -2 Fixes for review comments for Patch 41
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
2:36 p.m.
New subject: [PATCH] 41 -2 Fixes for review comments for Patch 41
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.
4236
days inactive
4240
days old
4 comments
2 participants
participants (2)
-
Abhishek Koneru -
Endi Sukma Dewata