Attila,
I applied third version of your patch. I have two comments:
- I don't see where you execute interoperablejs tests, I see them in
source code but it looks like they are not run as part of junit-all
target Maybe I missed something :(
- In CachingModuleScriptProviderBase, in line 70, you return null when
module source is null, according to CommonJS spec we should throw error
when module was not found
It looks great after your last refactoring :)
Regards,
Jarek
Attila Szegedi pisze:
Actually, I was informed on the commonjs list that there's a new set of compliance
tests at <http://github.com/kriskowal/commonjs/tree/master/tests/modules/1.0/>
and the one on interoperablejs is obsolete.
Also, just this morning i put out a new patch at
<https://bugzilla.mozilla.org/show_bug.cgi?id=540724> that already has all the
tests I wrote in it - this likely makes your work unnecessary; I'm sorry I didn't
wait on you, but I have scarce spare time to work on this, so when I do have, I take
advantage of it... By all means though, give it a review. I did end up taking your
approach and including the files, as the new set of files wasn't available through
command line SVN...
Anyway, others might want to take the opportunity to check the current patch -
it's now tested with 60% coverage; the untested stuff is mostly to do with
cache revalidation. It passes all the compliance tests + my additional
implementation tests.
Also, a big improvement compared to the previous patch is that I made require()
threadsafe, with good concurrent use properties, while not sacrificing
performance for single threaded use. So it can now be used to load modules on
demand into a shared top-level scope (which is something many server-side JS
embeddings do).
Attila.
--
home: http://www.szegedi.org
twitter: http://twitter.com/szegedi
weblog: http://constc.blogspot.com
On 2010.02.10., at 12:31, Jarosław Pałka wrote:
_______________________________________________
dev-tech-js-engine-rhino mailing list
[email protected]
https://lists.mozilla.org/listinfo/dev-tech-js-engine-rhino
