[mod-gnutls-devel] git commit review

[Daniel Kahn Gillmor]

I'm trying to clean up the scattered repositories we've got right now,
and two of the sourceforge ones seem to diverge.

I see the following changesets on
git://git.code.sf.net/p/mod-gnutls/code, but not on
git://mod-gnutls.git.sourceforge.net/gitroot/mod-gnutls/modgnutls.  some
of them look good, but some don't make sense to me.  Dash, can you
clarify some things?

* d334f47 Changed pre_connection Hook Order [Try to run before  mod_proxy]

I don't know what this fixes.  Can you add an example test that fails
without this change, and then show that this change resolves the
problem?

* d32dba4 Change module's contexts for configuration directives

The commit message here tells what happens, but not why.  Why are these
being changed?

* 5f610d0 Added apr_optional.h required for optional functions

What function(s) require this change?  Why add it if the code works
without it?

* d51befd Changed Default Export Of Full PEM Certificates To FALSE

This looks good to me, because it brings the code in line with the
documentation. 

* 63f19cb Changed gnutls_sign_algorithm_get_name() [non-existent in 2.12.x] to gnutls_sign_get_name() URL: http://gnutls.org/reference/gnutls-gnutls.html#gnutls-sign-get-name

I'm not sure i understand.  I see gnutls_sign_algorithm_get_name() in
gnutls 2.12.x:

0 dkg at alice:~$ grep -r gnutls_sign_algorithm_get_name /usr/include/gnutls
/usr/include/gnutls/gnutls.h:  const char *gnutls_sign_algorithm_get_name (gnutls_sign_algorithm_t sign);
0 dkg at alice:~$ dpkg -l libgnutls-dev
Desired=Unknown/Install/Remove/Purge/Hold
| Status=Not/Inst/Conf-files/Unpacked/halF-conf/Half-inst/trig-aWait/Trig-pend
|/ Err?=(none)/Reinst-required (Status,Err: uppercase=bad)
||/ Name           Version      Architecture Description
+++-==============-============-============-=================================
ii  libgnutls-dev  2.12.23-1    amd64        GNU TLS library - development fil
0 dkg at alice:~$ 

What makes you think the function isn't supported?  It hasn't been
marked deprecated from what i can tell.

Any thoughts or feedback on these changes?

One other note about git commit messages: the first line of the git
commit message is treated quite differently than all the others.  It's
good to make it a nice concise summary of the commit, and leave detailed
explanations and URLs and things to the rest of the (nicely-formatted)
commit message.  tpope's blog here has more detail:

   http://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html

I'm going to make an effort to try to stay with this style of commit
message where possible.

       --dkg
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 965 bytes
Desc: not available
URL: </pipermail/attachments/20130405/61a825c7/attachment.sig>