Code review comment for lp:~adiroiban/launchpad/bug-540105

Revision history for this message
Edwin Grubbs (edwin-grubbs) wrote :

> > Why are you calling LPS.use('lp.pofile', ...) repeatedly? This
> > should have no effect unless there is something seriously wrong with
> > lp.pofile.
> I moved them back in the same sandbox.
> I split independent functions in different sandboxes so that the failure of
> one function to not affect the others.

Hi Adi,

All your changes look good. Now I understand what you were trying to do with the separate LPS calls. Since I wasn't completely certain how far up the stack an exception goes before it continues running the rest of the javascript in the page, I played around with it a little bit. Apparently, an exception halts execution of a <script> block, so you would have to have multiple <script> blocks around each LPS.use() in order to ensure that the following code is run despite an exception above it. A much clearer way to handle that would be:

LPS.use('lp.pofile', function(Y) {
  try {
    do_something();
  } catch (e) {
    Y.log(e, "error");
  }
  do_something_else();
});

At least in firefox, if you call Y.log() with the second argument as "error", you will get a full stack trace in the console just like a normal uncaught exception.

Using try/catch is completely up to you.

 merge-approved

BTW, calling LPS.use() multiple times does not give you separate sandboxes, since you are re-using LPS instead of creating a new YUI() object.

-Edwin

review: Approve (code)

« Back to merge proposal