LGTM

https://codereview.appspot.com/10341043/diff/1/src/com/google/caja/ses/repairES5.js
File src/com/google/caja/ses/repairES5.js (right):

https://codereview.appspot.com/10341043/diff/1/src/com/google/caja/ses/repairES5.js#newcode222
src/com/google/caja/ses/repairES5.js:222: sev.level >=
ses.severities.MAGICAL_UNICORN.level &&
On 2013/06/17 19:53:07, felix8a wrote:
On 2013/06/17 17:50:42, kpreid2 wrote:
> I propose introducing the following constants next to the definition
of
> ses.severities.
> var LOWEST_SEVERITY = ses.severities.MAGICAL_UNICORN;
> var HIGHEST_SEVERITY = ses.severities.NOT_SUPPORTED;
> These mean that future revisions will be more likely to preserve the
correctness
> of this code without needing an equivalent of this CL.

I think that's a bad idea because it obscures why the comparison is >=
LOWEST
but < HIGHEST. It's deceptively similar to a half-open interval, which
it isn't.

Ah, I misread the code. I still think that we should use a
LOWEST_SEVERITY for the >=, but I agree that "< NOT_SUPPORTED" should be
left as is.

https://codereview.appspot.com/10341043/

--

--- You received this message because you are subscribed to the Google Groups "Google Caja Discuss" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.


Reply via email to