git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <junkio@cox.net>
To: Jakub Narebski <jnareb@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 0/7] gitweb: Cleanups, fixes and small improvements
Date: Sat, 26 Aug 2006 13:58:08 -0700	[thread overview]
Message-ID: <7vd5an1afz.fsf@assigned-by-dhcp.cox.net> (raw)
In-Reply-To: <1156612392716-git-send-email-jnareb@gmail.com> (Jakub Narebski's message of "Sat, 26 Aug 2006 19:13:12 +0200")

Jakub Narebski <jnareb@gmail.com> writes:

* gitweb: Restore old git_blame using git-annotate under "annotate"

  I actually was hoping to see a patch to remove the git_blame
  which is not used as far as I can see.

  Although we carried annotate and blame in git.git tree for
  quite a while, the intention was always to deprecate one over
  the other once pros-and-cons of each implementation become
  clear (and I think people would not miss annotate if we remove 
  annotate and make it an alias for blame -c anymore).  What's
  the reason we would want to have both?

* gitweb: Remove workaround for git-diff bug fixed in f82cd3c
. gitweb: Fix typo in git_patchset_body

  I think you had separate patches for these; applied.

* gitweb: Use 'local $/ = undef;' before 'print <$fd>;'

  You changed:

	$/ = undef;
	print <$fd>;
        ... hope that nobody depends on standard value of $/
	... around here, which may still break if you did sub
	... calls, the sub did not localize $/ (who would?),
        ... and depended to have a sane $/.
	$/ = "\n";

  to

	local $/ = undef;
	print <$fd>;
        ... hope that nobody depends on standard value of $/
	... until the end of scope, and whoever changes this
        ... sub is careful enough in the future

  which I think is worse.  Introducing an extra scope explicitly
  delimit the part you want to use localized $/ like this

	{ local $/; print <$fd>; } 

  might have been more palatable.  Am I guessing the reason of
  your change wrong?

* gitweb: Always link to plain version of the blob

  Needs a better description to justify the change by describing
  in what way the current implementation is broken.

The ones above I won't apply just yet (I first applied, merged
into "next" and then commented on the earlier 19 series, but I'm
not going to repeat that mistake again this time).

The rest look OK so I'll apply them; [6/7] depended on [5/7] so
I manually adjusted the context.

  parent reply	other threads:[~2006-08-26 20:58 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-08-26 17:13 [PATCH 0/7] gitweb: Cleanups, fixes and small improvements Jakub Narebski
2006-08-26 17:14 ` [PATCH 1/7] gitweb: Restore old git_blame using git-annotate under "annotate" Jakub Narebski
2006-08-26 17:14 ` [PATCH 2/7] gitweb: Remove workaround for git-diff bug fixed in f82cd3c Jakub Narebski
2006-08-26 17:14 ` [PATCH 3/7] gitweb: Improve comments about gitweb features configuration Jakub Narebski
2006-08-26 17:14 ` [PATCH 4/7] gitweb: Fix typo in git_patchset_body Jakub Narebski
2006-08-26 17:14 ` [PATCH 5/7] gitweb: Use 'local $/ = undef;' before 'print <$fd>;' Jakub Narebski
2006-08-26 17:14 ` [PATCH 6/7] gitweb: blobs defined by non-textual hash ids can be cached Jakub Narebski
2006-08-26 17:14 ` [PATCH 7/7] gitweb: Always link to plain version of the blob in git_blob Jakub Narebski
2006-08-26 20:58 ` Junio C Hamano [this message]
2006-08-26 21:18   ` [PATCH 0/7] gitweb: Cleanups, fixes and small improvements Jakub Narebski
2006-08-27 20:21   ` Jakub Narebski

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=7vd5an1afz.fsf@assigned-by-dhcp.cox.net \
    --to=junkio@cox.net \
    --cc=git@vger.kernel.org \
    --cc=jnareb@gmail.com \
    /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).