All of lore.kernel.org
 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 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.