Removed the trailing whitespace. Pushed to master. --Abhishek On Mon, 2013-03-25 at 14:32 -0400, Endi Sukma Dewata wrote:
Looks good to me. There's a trailing whitespace in the patch. Please fix before push.
-- Endi S. Dewata
----- Original Message -----
Please do a quick review of the attached patch with fixes made as per our discussion on IRC. Though Ade has ack'ed, would like to have the patch go through a final review as you have pointed out a mistake in the file path.
The comments were: 1. The file path specified where the user configuration is stored is wrong. * It is corrected from /var/lib/pki/<instance name>/<subsystem>/deployment.cfg to /etc/sysconfig/pki/tomcat/<instance name>/<subsystem>/deployment.cfg.
2. Removed the line : "-- When the -f <configuration filename> is not specified:".
3. Corected the extraspaces in the line HTTP port under Tomcat instance section
4. Capitalize the HTTP, AJP, HTTPS in the description of those fields.
5. Proper formatting of commit message - Done.
--Abhishek
On Wed, 2013-03-20 at 09:07 -0400, Abhishek Koneru wrote:
Please find the fixes for review comments given for patch 37 attached for review.
--Abhishek
On Tue, 2013-03-19 at 10:15 -0400, Ade Lee wrote:
Comments:
1. The commit message looks like it goes on continuously on a single line. This makes it difficult to see when doing a "git log".
2. The patch ends up adding several trailing whitespace errors. Fix these before checking in.
3. In pkidestroy man page, add the note about prompting for the parameter under each parameter, rather than as a separate note.
For example, -s <subsystem> Specifies the subsystem to be removed, where <subsystem> is CA, KRA, OCSP, or TKS. If this option is not specified, pkidestroy will prompt for its value.
4. Include documentation of the new password/ user options for pkidestroy. You can do this in the other ticket you are working on.
5. pkispawn:
a) Move the section on interactive mode to before examples. It should be a heading at the same level as OPTIONS and EXAMPLES b) Replace Dogtag with Certificate Server (all instances) c) The subheadings do not look right. Change as follows. Note that I have removed the enclosing brackets. These headings should be in bold.
[Choosing a subsystem type to install] --> Subsystem Type [Providing instance specific parameters] --> Instance Specific Parameters [Setting up the Administrator user] --> Administrative User Parameters [Specify the 389 Directory Server to be used for the internal database]-> Directory Server Parameters [Setting up the Security Domain properties]-> Security Domain Parameters
d) The security domain part should look like this:
Security Domain Parameters
Name: the name of the security domain. Required only if installing a root CA. Default value: <DNS domain name> Security Domain.
Hostname: the hostname for the security domain CA. Required for all non-CA subsystems. The default value is the hostname of this system. Secure HTTP port: the https port for the security domain. Required for all non-CA subsystems. The default value is 8443. Username: the username of the security domain administrator. Required for all non-CA subsystems. The default value is caadmin. Password: password for the security domain administrator. Required for all non-CA subsystems.
e) Do not bold or italicize any default values. [8080] -> 8080
On Fri, 2013-03-15 at 11:00 -0400, Abhishek Koneru wrote:
Please review the attached patch w.r.t the trac ticket 471 to add information regarding interactive mode installation to pkispawn/pkidestroy man pages.
--Abhishek _______________________________________________ Pki-devel mailing list Pki-devel@redhat.com https://www.redhat.com/mailman/listinfo/pki-devel
_______________________________________________ Pki-devel mailing list Pki-devel@redhat.com https://www.redhat.com/mailman/listinfo/pki-devel