git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Pierre Habouzit <madcoder@debian.org>
To: Junio C Hamano <junkio@cox.net>
Cc: git@vger.kernel.org
Subject: Re: [PATCH] git-daemon: more powerful base-path/user-path settings, using formats.
Date: Sun, 27 Aug 2006 12:28:09 +0200	[thread overview]
Message-ID: <200608271228.09718.madcoder@debian.org> (raw)
In-Reply-To: <7vfyfiyaex.fsf@assigned-by-dhcp.cox.net>

[-- Attachment #1: Type: text/plain, Size: 1586 bytes --]

Le dim 27 août 2006 08:12, Junio C Hamano a écrit :
> Pierre Habouzit <madcoder@debian.org> writes:
> > Allow a form of virtualhosting, when %h format is used.
> >
> > Signed-off-by: Pierre Habouzit <madcoder@debian.org>
> > ---
> >
> >     This is intended to be a more flexible solution, that also
> > gives virtual hosting as a bonus.
>
> Nicely done, almost.
>
> Having to have the distinction between %p and %P formats feels
> somewhat unwieldy, though.  Not that I have a better suggestion.
>
> > +int is_base_path_fmt;
> > +int is_user_path_fmt;
>
> I prefer these to be of type "static int".

omg, how did I missed that.

> Although I am not an authority of variable naming, these sound
> funny to me.  "is_XXX()" as a function name feels natural,
> "is_XXX" as a variable name does not --- it is not clear what
> the predicate is talking about.
>
> Maybe "use_fmt_for_base_path" is easier to understand?  I dunno.
> Or "user_path_is_fmt"?  That's more logical but still somewhat
> feels funny.

agreed.

There is also a second patch that never made it to the list that fixes:
 * some indentation problems due to a bad vimrc
 * --default-hostname switch (to handle virtual hosts even with older
   clients)
 * possible overflow in the formatting method.

I'll recompute a new patch that superseeds that one, and merge your 
comments and my never sent patch too.

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

  reply	other threads:[~2006-08-27 10:28 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-08-23 18:52 [PATCH] git-daemon virtual hosting implementation Pierre Habouzit
2006-08-23 20:11 ` Junio C Hamano
2006-08-23 20:56   ` Pierre Habouzit
2006-08-24 20:15     ` Jon Loeliger
2006-08-24 20:35       ` Junio C Hamano
2006-08-24 20:34         ` Jon Loeliger
2006-08-23 23:32   ` [PATCH] git-daemon: more powerful base-path/user-path settings, using formats Pierre Habouzit
2006-08-24  0:17     ` Junio C Hamano
2006-08-24  7:50       ` Pierre Habouzit
2006-08-27  6:12     ` Junio C Hamano
2006-08-27 10:28       ` Pierre Habouzit [this message]
2006-08-27 10:52         ` Junio C Hamano
2006-08-27 11:40           ` Pierre Habouzit
2006-08-27 15:30             ` Jakub Narebski
2006-08-27 16:26               ` Pierre Habouzit
2006-08-27 16:06           ` Randal L. Schwartz
  -- strict thread matches above, loose matches on Subject: below --
2006-08-27 21:49 Jon Loeliger

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=200608271228.09718.madcoder@debian.org \
    --to=madcoder@debian.org \
    --cc=git@vger.kernel.org \
    --cc=junkio@cox.net \
    /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).