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.
next prev 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).