Patch forthcoming.
Created attachment 32252 [details] patch
Comment on attachment 32252 [details] patch Nevermind. That doesn't compile. :(
Created attachment 32294 [details] patch
Created attachment 32295 [details] The whole file Eric suggested that I group the functions before moving them. The diff was kind of hard to read, so I'm attaching the whole file.
Comment on attachment 32294 [details] patch Your comment format is a bit strange. I would not have added all the crazy //////. It would make sense to me to describe each of these new classes some, more than just putting their names there. // Move to V8Constants, a class to hold all the constants used by V8? // Move to V8EventListener, a subclass of EventListener with v8 specific methods.
The r- is for not really knowing what all these new files do. The change itself is fine in idea... but it seems like we should document where these are going (if you want a real review at least).
Ok. This kind of patch seems hard to discuss via this system. Maybe it will be better to talk with Dimitri and Mike in person and come up with a plan and then execute the plan. Some of the groupings are really obvious. For example, the GC methods have virtually no overlap in concepts with the rest of the class. I think I went a bit overboard trying to find a home for everything in this patch. Thanks for the review.
Yeah, I wasn't really gonna review where you were gonna put stuff. So much as just that whoever next reads those comments should have some more idea than just the new class name. :) Also, I am alergic to *Utilities classes and *Helper classes :)
> Also, I am alergic to *Utilities classes and *Helper classes :) Oddly, V8Utilities is the only one of the bunch that currently exists! There might be a better place for the those exception idioms. V8ExceptionIdioms.h? :)
How about we start small? * Move event-related stuff to V8CustomEventListener * Move V8Executive-slated things to ScriptController?
(In reply to comment #10) > How about we start small? Ok. I'm actually just working on the GCController stuff atm. That looks pretty self-contained.
(In reply to comment #11) > (In reply to comment #10) > > How about we start small? > > Ok. I'm actually just working on the GCController stuff atm. That looks > pretty self-contained. Sounds great!