From: Jan Wielemaker <wielemak@science.uva.nl>
To: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Cc: Git Mailing List <git@vger.kernel.org>
Subject: Re: [PATCH] git-shell and git-cvsserver
Date: Mon, 8 Oct 2007 16:22:33 +0200 [thread overview]
Message-ID: <200710081622.33222.wielemak@science.uva.nl> (raw)
In-Reply-To: <Pine.LNX.4.64.0710080534270.4174@racer.site>
Dscho,
On Monday 08 October 2007 06:51, Johannes Schindelin wrote:
> On Fri, 5 Oct 2007, Jan Wielemaker wrote:
> > Hi,
> >
> > I know, I shouldn't be using git-cvsserver :-( Anyway, I patched
> > git-shell to start git-cvsserver if it is started interactively and the
> > one and only line given to it is "cvs server".
> >
> > The patch to shell.c is below. The trick with the EXEC_PATH is needed
> > because git-cvsserver doesn't appear to be working if you do not include
> > the git bindir in $PATH. I think that should be fixed in git-cvsserver
> > and otherwise we should at least make the value come from the prefix
> > make variable. With this patch I was able to use both Unix and Windows
> > cvs clients using git-shell as login shell.
> >
> > Note that you must provide ~/.gitconfig with user and email in the
> > restricted environment.
> >
> > Enjoy --- Jan
>
> I think this is a valuable contribution. That's why I comment...
Great :-)
> Please put a useful commit message (less like an email, more like
> something you want to read in git-log) at the beginning of the email, then
> a line containing _just_ "---", and after that some comments that are not
> meant to be stored in the history, like (I know this does not belong
> to...)
>
> After that, there should be a diffstat, and then the patch.
>
> The easiest to have this layout is to do a proper commit in git, use "git
> format-patch" to produce the patch, and then insert what you want to say
> in addition to the commit message between the "---" marker and the
> diffstat.
I buy that. I'm still trying to get used to all the features ...
> I strongly disagree (as you yourself, probably) with the notion that this
> does not belong into git-shell.
>
> > +#define EXEC_PATH "/usr/local/bin"
>
> This is definitely wrong. Use git_exec_path() instead.
I stated in my comments I was not happy about that. Before I start
submitting a new patch that may or may not be accepted, I'd like to have
some things clear. I manage an Open Source project for a long time (the
term not even invented in 1985 :-) Users come up with problems and
report on this. Most often with just a statement they don't like it,
sometimes with a detailed description of what is wrong and how to fix
it, sometimes with patches.
Generally, patches are not really how I like them, precisely the kind
of things that are wrong with my patch. Style issues, fixed A where
I consider the patch must be in B after a conflict between A and B
was detected, missing opportunities for code reuse, etc.
I try to talk frequent contributors into making things as `ready-to-use'
as possible. For incidental contributors I generally don't care. I just
rewrite the patch. Its less work for me than trying to explain all
details (as you kindly did and I agree to most of it, even learn some
new things) and it is too much work for someone who wishes an incidental
patch in the main distribution.
I don't want to become a GIT comitter, and I don't want to learn all of
its internals. Just a happy user for the SWI-Prolog project and an
internal project with some CVS addicts.
Cheers --- Jan
next prev parent reply other threads:[~2007-10-08 14:28 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-10-05 12:53 [PATCH] git-shell and git-cvsserver Jan Wielemaker
2007-10-05 14:31 ` Miklos Vajna
2007-10-05 15:07 ` Frank Lichtenheld
2007-10-08 4:51 ` Johannes Schindelin
2007-10-08 14:22 ` Jan Wielemaker [this message]
2007-10-09 11:51 ` Johannes Schindelin
2007-10-09 14:33 ` [PATCH] Support cvs via git-shell Johannes Schindelin
2007-10-09 22:35 ` Frank Lichtenheld
2007-10-10 13:29 ` Johannes Schindelin
2007-10-10 19:10 ` Frank Lichtenheld
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=200710081622.33222.wielemak@science.uva.nl \
--to=wielemak@science.uva.nl \
--cc=Johannes.Schindelin@gmx.de \
--cc=git@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).