[Pki-devel] [PATCH] 37-3 Fixes for review comments on [PATCH] 37 - 2

Abhishek Koneru akoneru at redhat.com
Mon Mar 25 20:13:40 UTC 2013


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 at redhat.com
> > > > > https://www.redhat.com/mailman/listinfo/pki-devel
> > > > 
> > > > 
> > > 
> > > _______________________________________________
> > > Pki-devel mailing list
> > > Pki-devel at redhat.com
> > > https://www.redhat.com/mailman/listinfo/pki-devel
> > 
> > 
> > 





More information about the Pki-devel mailing list