On Fri, Nov 08, 2013 at 12:07:22PM +0100, Chris Leick wrote:
> Hi Jakub,
> 
> Chris Leick:
> >Hi Jakub,
> >>>
> >>>Would you like to submit a patch for the bugs that come from
> >>>the man pages
> >>>or would you prefer if we did it? The man pages are XML
> >>>DocBook documents
> >>>located in src/man in the SSSD checkout.
> >
> >Ok. I will sent patches.
> 
> You can apply the patch attached to the xml files.
> 
> Kind regards,
> Chris

Thank you, see a couple of remarks in-line.

> diff -ru sssd/src/man/include/ldap_id_mapping.xml 
> sssd_new/src/man/include/ldap_id_mapping.xml
> --- sssd/src/man/include/ldap_id_mapping.xml  2013-11-08 11:23:39.313847600 
> +0100
> +++ sssd_new/src/man/include/ldap_id_mapping.xml      2013-11-08 
> 11:49:49.849801198 +0100
> @@ -168,7 +168,7 @@
>                          <para>
>                              When this option is configured, domains will be
>                              allocated starting with slice zero and increasing
> -                            monatomically with each additional domain.
> +                            monotomically with each additional domain.

I think the right spelling here would be "monotonically".

>                          </para>
>                          <para>
>                              NOTE: This algorithm is non-deterministic (it
> diff -ru sssd/src/man/include/seealso.xml sssd_new/src/man/include/seealso.xml
> --- sssd/src/man/include/seealso.xml  2013-11-08 11:23:39.313847600 +0100
> +++ sssd_new/src/man/include/seealso.xml      2013-11-08 12:01:49.941779917 
> +0100
> @@ -2,25 +2,19 @@
>          <title>SEE ALSO</title>
>          <para>
>              <citerefentry>
> -                <refentrytitle>sssd</refentrytitle><manvolnum>8</manvolnum>
> -            </citerefentry>,
> -            <citerefentry>
> -                
> <refentrytitle>sssd.conf</refentrytitle><manvolnum>5</manvolnum>
> +                
> <refentrytitle>sssd-ad</refentrytitle><manvolnum>5</manvolnum>
>              </citerefentry>,
>              <citerefentry>
> -                
> <refentrytitle>sssd-ldap</refentrytitle><manvolnum>5</manvolnum>
> +                
> <refentrytitle>sssd-ipa</refentrytitle><manvolnum>5</manvolnum>
>              </citerefentry>,
>              <citerefentry>
>                  
> <refentrytitle>sssd-krb5</refentrytitle><manvolnum>5</manvolnum>
>              </citerefentry>,
>              <citerefentry>
> -                
> <refentrytitle>sssd-simple</refentrytitle><manvolnum>5</manvolnum>
> -            </citerefentry>,
> -            <citerefentry>
> -                
> <refentrytitle>sssd-ipa</refentrytitle><manvolnum>5</manvolnum>
> +                
> <refentrytitle>sssd-ldap</refentrytitle><manvolnum>5</manvolnum>
>              </citerefentry>,
>              <citerefentry>
> -                
> <refentrytitle>sssd-ad</refentrytitle><manvolnum>5</manvolnum>
> +                
> <refentrytitle>sssd-simple</refentrytitle><manvolnum>5</manvolnum>
>              </citerefentry>,
>              <phrase condition="with_sudo">
>                  <citerefentry>
> @@ -29,6 +23,12 @@
>                  </citerefentry>,
>              </phrase>
>              <citerefentry>
> +                
> <refentrytitle>sssd.conf</refentrytitle><manvolnum>5</manvolnum>
> +            </citerefentry>,
> +            <citerefentry>
> +                
> <refentrytitle>pam_sss</refentrytitle><manvolnum>8</manvolnum>
> +            </citerefentry>,
> +            <citerefentry>
>                  
> <refentrytitle>sss_cache</refentrytitle><manvolnum>8</manvolnum>
>              </citerefentry>,
>              <citerefentry>
> @@ -41,19 +41,10 @@
>                  
> <refentrytitle>sss_groupdel</refentrytitle><manvolnum>8</manvolnum>
>              </citerefentry>,
>              <citerefentry>
> -                
> <refentrytitle>sss_groupshow</refentrytitle><manvolnum>8</manvolnum>
> -            </citerefentry>,
> -            <citerefentry>
>                  
> <refentrytitle>sss_groupmod</refentrytitle><manvolnum>8</manvolnum>
>              </citerefentry>,
>              <citerefentry>
> -                
> <refentrytitle>sss_useradd</refentrytitle><manvolnum>8</manvolnum>
> -            </citerefentry>,
> -            <citerefentry>
> -                
> <refentrytitle>sss_userdel</refentrytitle><manvolnum>8</manvolnum>
> -            </citerefentry>,
> -            <citerefentry>
> -                
> <refentrytitle>sss_usermod</refentrytitle><manvolnum>8</manvolnum>
> +                
> <refentrytitle>sss_groupshow</refentrytitle><manvolnum>8</manvolnum>
>              </citerefentry>,
>              <citerefentry>
>                  
> <refentrytitle>sss_obfuscate</refentrytitle><manvolnum>8</manvolnum>
> @@ -61,9 +52,6 @@
>              <citerefentry>
>                  
> <refentrytitle>sss_seed</refentrytitle><manvolnum>8</manvolnum>
>              </citerefentry>,
> -            <citerefentry>
> -                
> <refentrytitle>sssd_krb5_locator_plugin</refentrytitle><manvolnum>8</manvolnum>
> -            </citerefentry>,
>              <phrase condition="with_ssh">
>                  <citerefentry>
>                      <refentrytitle>sss_ssh_authorizedkeys</refentrytitle>
> @@ -75,7 +63,19 @@
>                  </citerefentry>,
>              </phrase>
>              <citerefentry>
> -                
> <refentrytitle>pam_sss</refentrytitle><manvolnum>8</manvolnum>
> +                
> <refentrytitle>sss_useradd</refentrytitle><manvolnum>8</manvolnum>
> +            </citerefentry>,
> +            <citerefentry>
> +                
> <refentrytitle>sss_userdel</refentrytitle><manvolnum>8</manvolnum>
> +            </citerefentry>,
> +            <citerefentry>
> +                
> <refentrytitle>sss_usermod</refentrytitle><manvolnum>8</manvolnum>
> +            </citerefentry>,
> +            <citerefentry>
> +                <refentrytitle>sssd</refentrytitle><manvolnum>8</manvolnum>
> +            </citerefentry>,
> +            <citerefentry>
> +                
> <refentrytitle>sssd_krb5_locator_plugin</refentrytitle><manvolnum>8</manvolnum>
>              </citerefentry>.
>          </para>
>      </refsect1>

These changes just re-order the list, right?

> diff -ru sssd/src/man/pam_sss.8.xml sssd_new/src/man/pam_sss.8.xml
> --- sssd/src/man/pam_sss.8.xml        2013-11-08 11:23:39.313847600 +0100
> +++ sssd_new/src/man/pam_sss.8.xml    2013-11-08 11:44:11.721811179 +0100
> @@ -23,19 +23,19 @@
>          <cmdsynopsis>
>              <command>pam_sss.so</command>
>              <arg choice='opt'>
> -                <replaceable>quiet</replaceable>
> +                <literal>quiet</literal>
>              </arg>
>              <arg choice='opt'>
> -                <replaceable>forward_pass</replaceable>
> +                <literal>forward_pass</literal>
>              </arg>
>              <arg choice='opt'>
> -                <replaceable>use_first_pass</replaceable>
> +                <literal>use_first_pass</literal>
>              </arg>
>              <arg choice='opt'>
> -                <replaceable>use_authtok</replaceable>
> +                <literal>use_authtok</literal>
>              </arg>
>              <arg choice='opt'>
> -                <replaceable>retry=N</replaceable>
> +                <literal>retry=N</literal>

Sorry, I don't think this change is correct, I can't build the man page
with these elements:

/home/remote/jhrozek/devel/sssd/src/man/pam_sss.8.xml:25: element arg:
validity error : Element literal is not declared in arg list of possible
children
/home/remote/jhrozek/devel/sssd/src/man/pam_sss.8.xml:28: element arg:
validity error : Element literal is not declared in arg list of possible
children
/home/remote/jhrozek/devel/sssd/src/man/pam_sss.8.xml:31: element arg:
validity error : Element literal is not declared in arg list of possible
children
/home/remote/jhrozek/devel/sssd/src/man/pam_sss.8.xml:34: element arg:
validity error : Element literal is not declared in arg list of possible
children
/home/remote/jhrozek/devel/sssd/src/man/pam_sss.8.xml:37: element arg:
validity error : Element literal is not declared in arg list of possible
children

>              </arg>
>          </cmdsynopsis>
>      </refsynopsisdiv>
> diff -ru sssd/src/man/sssd-ad.5.xml sssd_new/src/man/sssd-ad.5.xml
> --- sssd/src/man/sssd-ad.5.xml        2013-11-08 11:23:39.337847600 +0100
> +++ sssd_new/src/man/sssd-ad.5.xml    2013-11-08 11:47:03.049806124 +0100
> @@ -72,7 +72,7 @@
>              Active Directory, you should set
>              <programlisting>
>  ldap_id_mapping = False
> -            </programlisting>
> +            </programlisting>.

When I built the man page, the dot appears on its own line:

   want to disable ID mapping and instead rely on POSIX attributes
   defined in Active Directory, you should set

      ldap_id_mapping = False

   . In order to retrieve users

>              In order to retrieve users and groups using POSIX attributes 
> from trusted
>              domains, the AD administrator must make sure that the POSIX 
> attributes
>              are replicated to the Global Catalog.
> diff -ru sssd/src/man/sssd.conf.5.xml sssd_new/src/man/sssd.conf.5.xml
> --- sssd/src/man/sssd.conf.5.xml      2013-11-08 11:23:39.337847600 +0100
> +++ sssd_new/src/man/sssd.conf.5.xml  2013-11-08 12:02:45.045778290 +0100
> @@ -181,7 +181,7 @@
>                              </para>
>                              <para>
>                                  Each domain can have an individual format 
> string configured.
> -                                see DOMAIN SECTIONS for more info on this 
> option.
> +                                See DOMAIN SECTIONS for more info on this 
> option.
>                              </para>
>                          </listitem>
>                      </varlistentry>
> @@ -239,7 +239,7 @@
>                              <para>
>                                  This string will be used as a default domain
>                                  name for all names without a domain name
> -                                component. The main use case is environments
> +                                component. The main use case are environments

I think the original was correct here, the sentence reads to me as the
singular is used together with "use case" not "environments". But I'm
not an English native speaker either..

>                                  where the primary domain is intended for 
> managing host
>                                  policies and all users are located in a 
> trusted domain.
>                                  The option allows those users
> @@ -269,7 +269,7 @@
>              Settings that can be used to configure different services
>              are described in this section. They should reside in the
>              [<replaceable>$NAME</replaceable>] section, for example,
> -            for NSS service, the section would be <quote>[nss]</quote>
> +            for NSS service, the section would be <quote>[nss]</quote>.
>          </para>
>  
>          <refsect2 id='general'>
> @@ -408,7 +408,7 @@
>                      <listitem>
>                          <para>
>                              How many seconds should nss_sss cache 
> enumerations
> -                            (requests for info about all users)
> +                            (requests for info about all users)?

I don't think the question mark looks good here, sorry.

>                          </para>
>                          <para>
>                              Default: 120
> @@ -918,8 +918,8 @@
>                      <listitem>
>                          <para>
>                              UID and GID limits for the domain. If a domain
> -                            contains an entry that is outside these limits, 
> it
> -                            is ignored.
> +                            contains an entry that is outside these limits, 
> this
> +                            entry is ignored.
>                          </para>
>                          <para>
>                              For users, this affects the primary GID limit. 
> The
> @@ -1352,7 +1352,7 @@
>                          <para>
>                              The access control provider used for the domain.
>                              There are two built-in access providers (in
> -                            addition to any included in installed backends)
> +                            addition to any included in installed backends).
>                              Internal special providers are:
>                          </para>
>                          <para>
> @@ -1853,7 +1853,7 @@
>          <refsect2 id='local_domain'>
>              <title>The local domain section</title>
>              <para>
> -                This section contains settings for domain that stores users 
> and
> +                This section contains settings for domains that stores users 
> and

I think there is one more change to be done, instead of "domains that
stores users", shouldn't it say "domains that store users" ?

>                  groups in SSSD native database, that is, a domain that uses
>                  <replaceable>id_provider=local</replaceable>.
>              </para>
> @@ -1962,7 +1962,7 @@
>                      <listitem>
>                          <para>
>                              The command that is run after a user is removed.
> -                            The command us passed the username of the user 
> being
> +                            The command is passed the username of the user 
> being
>                              removed as the first and only parameter. The 
> return
>                              code of the command is not taken into account.
>                          </para>
> diff -ru sssd/src/man/sssd-krb5.5.xml sssd_new/src/man/sssd-krb5.5.xml
> --- sssd/src/man/sssd-krb5.5.xml      2013-11-08 11:23:39.337847600 +0100
> +++ sssd_new/src/man/sssd-krb5.5.xml  2013-11-08 11:48:46.173803079 +0100
> @@ -348,7 +348,7 @@
>                              <emphasis>d</emphasis> for days.
>                          </para>
>                          <para>
> -                            If there is no unit given <emphasis>s</emphasis> 
> is
> +                            If there is no unit given, 
> <emphasis>s</emphasis> is
>                              assumed.
>                          </para>
>                          <para>
> @@ -423,7 +423,7 @@
>                          <para>
>                              <emphasis>demand</emphasis> to use FAST. The
>                              authentication fails if the server does not
> -                            require fast.
> +                            require FAST.
>                          </para>
>                          <para>
>                              Default: not set, i.e. FAST is not used.
> diff -ru sssd/src/man/sssd-ldap.5.xml sssd_new/src/man/sssd-ldap.5.xml
> --- sssd/src/man/sssd-ldap.5.xml      2013-11-08 11:23:39.337847600 +0100
> +++ sssd_new/src/man/sssd-ldap.5.xml  2013-11-08 11:42:11.945814725 +0100
> @@ -149,7 +149,7 @@
>                              The namingContexts attribute must have a
>                              single value with the DN of the search base of 
> the
>                              LDAP server to make this work. Multiple values 
> are
> -                            are not supported.
> +                            not supported.
>                          </para>
>                      </listitem>
>                  </varlistentry>
> @@ -356,7 +356,7 @@
>                          <para>
>                              The LDAP attribute that contains the objectSID of
>                              an LDAP user object. This is usually only
> -                            necessary for ActiveDirectory servers.
> +                            necessary for Active Directory servers.
>                          </para>
>                          <para>
>                              Default: objectSid for ActiveDirectory, not set
> @@ -705,7 +705,7 @@
>                      <term>ldap_user_authorized_service (string)</term>
>                      <listitem>
>                          <para>
> -                            If access_provider=ldap and
> +                            If using access_provider=ldap and

Hm, the original sentence looked OK to me, the "using" didn't sound good
to you?

>                              ldap_access_order=authorized_service, SSSD will
>                              use the presence of the authorizedService
>                              attribute in the user's LDAP entry to determine
> @@ -733,7 +733,7 @@
>                      <term>ldap_user_authorized_host (string)</term>
>                      <listitem>
>                          <para>
> -                            If access_provider=ldap and
> +                            If using access_provider=ldap and

Same here

>                              ldap_access_order=host, SSSD will use the 
> presence
>                              of the host attribute in the user's LDAP entry to
>                              determine access privilege.
> @@ -826,7 +826,7 @@
>                          <para>
>                              The LDAP attribute that contains the objectSID of
>                              an LDAP group object. This is usually only
> -                            necessary for ActiveDirectory servers.
> +                            necessary for Active Directory servers.
>                          </para>
>                          <para>
>                              Default: objectSid for ActiveDirectory, not set
> @@ -1038,7 +1038,7 @@
>                      <term>ldap_service_name (string)</term>
>                      <listitem>
>                          <para>
> -                            The LDAP attribute that contains the name of
> +                            The LDAP attribute that contains the names of
>                              service attributes and their aliases.
>                          </para>
>                          <para>
> @@ -1434,7 +1434,7 @@
>                  </varlistentry>
>  
>                  <varlistentry>
> -                    <term>ldap_min_id, ldap_max_id (interger)</term>
> +                    <term>ldap_min_id, ldap_max_id (integer)</term>
>                      <listitem>
>                          <para>
>                              In contrast to the SID based ID mapping which is


The rest of the changes looks good to me. Thanks again for improving the
documentation. Would it be OK for you to send a git-formatted patch in
the next iteration? If not, a diff is fine, I can apply it myself.
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Reply via email to