LGTM
http://codereview.chromium.org/123021/diff/1/7 File src/debug-delay.js (right): http://codereview.chromium.org/123021/diff/1/7#newcode1564 Line 1564: // Get the selected frame. With no frame argument use the selected frame. Could you change one of the occurrences of selected in this comment to something else to make it less confusing? http://codereview.chromium.org/123021/diff/1/7#newcode1597 Line 1597: // Get the selected frame. With no frame argument use the selected frame. Ditto. http://codereview.chromium.org/123021/diff/1/7#newcode1599 Line 1599: if (request.arguments && !IS_UNDEFINED(request.arguments.frameNumber)) { Factor out this code out since it is shared by 'scope' and 'scopes' requests. http://codereview.chromium.org/123021/diff/1/5 File src/mirror-delay.js (right): http://codereview.chromium.org/123021/diff/1/5#newcode431 Line 431: * transient handle.. Two periods. The other param comments have no periods. http://codereview.chromium.org/123021/diff/1/5#newcode572 Line 572: * transient handle.. Ditto. http://codereview.chromium.org/123021/diff/1/5#newcode1650 Line 1650: * @param {Object} data The context data The param comment does not match the function. http://codereview.chromium.org/123021/diff/1/5#newcode1680 Line 1680: // created on the fly or materializing the local and closure scopes and Remove 'or'? http://codereview.chromium.org/123021/diff/1/3 File src/runtime.cc (right): http://codereview.chromium.org/123021/diff/1/3#newcode6114 Line 6114: ScopeInfo<> sinfo(*code); Spell out scope: sinfo -> scope_info? http://codereview.chromium.org/123021/diff/1/3#newcode6176 Line 6176: ScopeInfo<> sinfo(*code); scope_info? http://codereview.chromium.org/123021/diff/1/3#newcode6178 Line 6178: // Allocate and initialize a JSObject with all the XXX. XXX -> real comment http://codereview.chromium.org/123021/diff/1/3#newcode6200 Line 6200: // Fill all context locals to the context extension. It seems that the rest of this could be factored out shared with Materialize local scope? http://codereview.chromium.org/123021/diff/1/3#newcode6269 Line 6269: // If at the local scope mark the local scope as passed. the local scope -> a local scope? http://codereview.chromium.org/123021/diff/1/3#newcode6274 Line 6274: // If the current context is not associated with the local scope that is that is -> the current context is? http://codereview.chromium.org/123021 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
