On Mon, Dec 15, 2008 at 2:17 PM, Jakub Narebski <jnareb@xxxxxxxxx> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
