git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jakub Narebski <jnareb@gmail.com>
To: "J.H." <warthog9@eaglescrag.net>
Cc: git@vger.kernel.org
Subject: Re: [RFC PATCH 0/2] gitweb: die_error (error handling) improvements
Date: Mon, 13 Dec 2010 08:55:22 +0100	[thread overview]
Message-ID: <201012130855.23013.jnareb@gmail.com> (raw)
In-Reply-To: <4D058228.7040905@eaglescrag.net>

On Mon, 13 Dec 2010, J.H. wrote:
> On 12/12/2010 04:46 PM, Jakub Narebski wrote:

> > The following two patch series changes improve error / exception
> > handling in gitweb, preparing the way for gitweb output caching, but
> > useful even without it.
> > 
> > I'm sending this patch series early to gather feedback on possible
> > ways of improving error / exception handling in gitweb.
> 
> Personally, instead of another band-aid over this problem, and adding
> (or further legitimizing) goto statements inside gitweb I'd much *MUCH*
> rather we actually put in the work to actually clean this up.

That's not band-aid, that's using Perl exception mechanism.  Gitweb
uses die_error() like one would ordinarily use "die".
 
> This is the direction I'm heading in, which I mentioned in an earlier
> e-mail.

Well, then how do you want to handle errors?  Note that die_error calls
are sometimes nested quite deep in the call stack, so using return value
to denote errors and checking it is rather out of question: it would
significantly increase complexity of code for no gain.

Nevertheless I'll take a look how it is solved in other web applications
written in Perl, like SVN::Web or CPAN Hubble.
 
> There are a *LOT* of disadvantages to the eval mechanism in perl.  It's
> the standard but gitweb is getting more and more complex, and eval is
> simplistic.  Couple that with the complexity and uncertainty that things
> like goto add to the code, I would *MUCH* rather not see this series go
> in, as I think it is the wrong approach to fixing this.

eval / die is not like goto, but like exception mechanism in other
languages.  I'd prefer to use Try::Tiny or TryCatch, but we have this
"no extra dependencies" policy for gitweb.
-- 
Jakub Narebski
Poland

  reply	other threads:[~2010-12-13  7:55 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-12-13  0:46 [RFC PATCH 0/2] gitweb: die_error (error handling) improvements Jakub Narebski
2010-12-13  0:48 ` [RFC PATCH 1/2] gitweb: Go to DONE_REQUEST rather than DONE_GITWEB in die_error Jakub Narebski
2010-12-13  0:49 ` [RFC PATCH 2/2] gitweb: use eval + die for error (exception) handling Jakub Narebski
2010-12-13  2:17 ` [RFC PATCH 0/2] gitweb: die_error (error handling) improvements J.H.
2010-12-13  7:55   ` Jakub Narebski [this message]
2010-12-13  9:46     ` 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=201012130855.23013.jnareb@gmail.com \
    --to=jnareb@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=warthog9@eaglescrag.net \
    /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).