From: Jakub Narebski <jnareb@gmail.com>
To: Mark Rada <marada@uwaterloo.ca>
Cc: Junio C Hamano <gitster@pobox.com>, git@vger.kernel.org
Subject: Re: [PATCH v5 1/2] gitweb: check given hash before trying to create snapshot
Date: Thu, 1 Oct 2009 10:13:57 +0200 [thread overview]
Message-ID: <200910011013.58899.jnareb@gmail.com> (raw)
In-Reply-To: <4ABE5360.8090204@mailservices.uwaterloo.ca>
On Sat, 26 Sep 2009, Mark Rada wrote:
> Makes things nicer in cases when you hand craft the snapshot URL but
> make a typo in defining the hash variable (e.g. netx instead of next);
> you will now get an error message instead of a broken tarball.
>
> Tests for t9501 are included to demonstrate added functionality.
>
> Signed-off-by: Mark Rada <marada@uwaterloo.ca>
> ---
Very nice, and I think robust solution.
Acked-by: Jakub Narebski <jnareb@gmail.com>
[...]
> diff --git a/t/t9501-gitweb-standalone-http-status.sh b/t/t9501-gitweb-standalone-http-status.sh
> index d0ff21d..0688a57 100644
> --- a/t/t9501-gitweb-standalone-http-status.sh
> +++ b/t/t9501-gitweb-standalone-http-status.sh
BTW. the rest of test scripts are executable, but not this one? Why?
(But correcting this should be done, if needed, in separate commit).
> @@ -75,4 +75,43 @@ test_expect_success \
> test_debug 'cat gitweb.output'
>
>
> +# ----------------------------------------------------------------------
> +# snapshot hash ids
> +
> +test_expect_success 'snapshots: good tree-ish id' '
> + gitweb_run "p=.git;a=snapshot;h=master;sf=tgz" &&
> + grep "Status: 200 OK" gitweb.output
> +'
> +test_debug 'cat gitweb.output'
> +
What *could* be improved (but don't *need to*) is to check also HTTP
status and not only formatted error message:
> +test_expect_success 'snapshots: bad tree-ish id' '
> + gitweb_run "p=.git;a=snapshot;h=frizzumFrazzum;sf=tgz" &&
+ grep "Status: 404 Not Found" gitweb.output &&
> + grep "404 - Object does not exist" gitweb.output
> +'
> +test_debug 'cat gitweb.output'
And similarly in other cases. But it is not something required.
I think what it is now is good enough.
--
Jakub Narebski
Poland
prev parent reply other threads:[~2009-10-01 8:15 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-09-26 17:46 [PATCH v5 1/2] gitweb: check given hash before trying to create snapshot Mark Rada
2009-10-01 8:13 ` Jakub Narebski [this message]
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=200910011013.58899.jnareb@gmail.com \
--to=jnareb@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=marada@uwaterloo.ca \
/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).