Hi
I'm not sure if this is the right list, but here is my first GIT patch.
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.
Merry X-Mas.
--
mit freundlichen Grüßen / best regards
Gerhard Gappmeier
ascolab GmbH - automation system communication laboratory
Tel.: +49 9131 691 123
Fax: +49 9131 691 128
Web: http://www.ascolab.com
GPG Key-Id: 5AAC50C4
GPG Fingerprint: 967A 15F1 2788 164D CCA3 6C46 07CD 6F82 5AAC 50C4
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
[off-list ref] 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
On Thu, Dec 24, 2009 at 12:38 PM, Erik Faye-Lund
[off-list ref] wrote:
Anyway, this is all I bother to point out before I see an inlined,
white-space fixed patch.
Actually, I'll add one thing: using syslog in git-shell breaks the
Windows builds. I don't know about other platforms.
Currently, git-daemon is the only git-program that depends on syslog,
and it's excluded from Windows builds (but this is something that
might change soon).
--
Erik "kusma" Faye-Lund