Please read Documentation/SubmittingPatches. We prefer inline patches,
as they are easier to review.
On Wed, Dec 23, 2009 at 6:32 PM, Gerhard Gappmeier
<gerhard.gappmeier@xxxxxxxxxxx> wrote:
> Hi
>
> I'm not sure if this is the right list, but here is my first GIT patch.
>
It's the right list :)
> I had a problem with git-shell and wanted to analyze it.
> Unfortunately it does not contain any trace capabilities.
> So I cloned git and added some basic syslog support.
> After that I recognized that the current git version just works ;-)
> but the syslog functionality is always a nice thing I think.
> So here is the patch.
Looking at your patch, I see there's a lot of white-space changes. Stuff like:
> - const char *cvsserver_argv[3] = {
> - "cvsserver", "server", NULL
> - };
> + const char *cvsserver_argv[3] = {
> + "cvsserver", "server", NULL
> + };
just makes this harder to review. Besides, we use tabs for indentation in git.
Also, I think it would be better to use set_die_routine() from usage.h
than to change all the die call-sites. This is what git-daemon does:
--->8---
if (log_syslog) {
openlog("git-daemon", LOG_PID, LOG_DAEMON);
set_die_routine(daemon_die);
}
--->8---
Look at daemon.c for the implementation of daemon_die().
+/* Syslog defines */
+#define GIT_SYSLOG_IDENT "git-shell"
+#define GIT_SYSLOG_OPTION 0
+#define GIT_SYSLOG_FACILITY LOG_LOCAL0
+
Is this really needed? These are only used at one place. Just doing
+ openlog("git-shell", 0, LOG_LOCAL0);
would IMO be cleaner.
Anyway, this is all I bother to point out before I see an inlined,
white-space fixed patch.
> Merry X-Mas.
Happy holidays to you too :)
--
Erik "kusma" Faye-Lund
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html