From: "Giuseppe Bilotta" <giuseppe.bilotta@gmail.com>
To: "Jakub Narebski" <jnareb@gmail.com>
Cc: git@vger.kernel.org, "Petr Baudis" <pasky@suse.cz>,
"Junio C Hamano" <gitster@pobox.com>
Subject: Re: [RFCv4 1/3] gitweb: add patch view
Date: Mon, 15 Dec 2008 14:48:20 +0100 [thread overview]
Message-ID: <cb7bb73a0812150548w526a8c0eu13ec95785e0ab824@mail.gmail.com> (raw)
In-Reply-To: <200812151417.16372.jnareb@gmail.com>
On Mon, Dec 15, 2008 at 2:17 PM, Jakub Narebski <jnareb@gmail.com> wrote:
>> + my $patch_max;
>> + if ($format eq 'patch') {
>> + $patch_max = gitweb_check_feature('patches');
>> + die_error(403, "Patch view not allowed") unless $patch_max;
>> + }
>> +
>
> Hmmm... gitweb_check_feature
You're right, it's an abuse. I'll make it gitweb_get_feature()[0]
> I am wondering if we could somehow mark (encode) either $hash_parent
> or number of patches in proposed filename... but I don't think it is
> worth it.
Including hash_parent if defined is quite possible. I'm not sure it's
really worth it considering that the typical usage would be to publish
a patchset for a particular feature (in which case the hash/branch
name would be enough).
>> + } elsif ($format eq 'patch') {
>> + local $/ = undef;
>> + print <$fd>;
>> + close $fd
>> + or print "Reading git-format-patch failed\n";
>
> Nice, although... I'd prefer for Perl expert to say if it is better
> to dump file as a whole in such way (it might be quite large), or
> to do it line by line, i.e. without "local $/ = undef;", or using
> "print while <$fd>;" also without "local $/ = undef;".
I'm just sticking to whatever the existing code does :-)
As soon as you finish the patchset review I'll have a new version
taking into consideration all the other suggestions and remarks I
snipped from this reply.
--
Giuseppe "Oblomov" Bilotta
next prev parent reply other threads:[~2008-12-15 13:49 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-12-06 15:02 [RFCv4 0/3] gitweb: patch view Giuseppe Bilotta
2008-12-06 15:02 ` [RFCv4 1/3] gitweb: add " Giuseppe Bilotta
2008-12-06 15:02 ` [RFCv4 2/3] gitweb: add patches view Giuseppe Bilotta
2008-12-06 15:02 ` [RFCv4 3/3] gitweb: link to patch(es) view from commit and log views Giuseppe Bilotta
2008-12-16 1:03 ` Jakub Narebski
[not found] ` <cb7bb73a0812160202n1f4f7f4fi7f71455eb42bcd31@mail.gmail.com>
2008-12-16 10:14 ` Jakub Narebski
2008-12-16 11:14 ` Giuseppe Bilotta
2008-12-16 3:14 ` [RFCv4 2/3] gitweb: add patches view Jakub Narebski
[not found] ` <cb7bb73a0812160149j1dcaefccv1caf4a2e589ffebb@mail.gmail.com>
2008-12-16 10:16 ` Jakub Narebski
2008-12-15 13:17 ` [RFCv4 1/3] gitweb: add patch view Jakub Narebski
2008-12-15 13:48 ` Giuseppe Bilotta [this message]
2008-12-15 13:58 ` Jakub Narebski
[not found] <20081207060430.GE4357@ftbfs.org>
2008-12-07 9:52 ` Giuseppe Bilotta
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=cb7bb73a0812150548w526a8c0eu13ec95785e0ab824@mail.gmail.com \
--to=giuseppe.bilotta@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=jnareb@gmail.com \
--cc=pasky@suse.cz \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.