Hi Yang
Thank you for review. I'm sorry for a half-done patch. Hope this one is
better
plus it there is a test.
Peter
http://codereview.chromium.org/10544151/diff/1/src/debug-debugger.js
File src/debug-debugger.js (right):
http://codereview.chromium.org/10544151/diff/1/src/debug-debugger.js#newcode2407
src/debug-debugger.js:2407: // No frames no evaluate in frame.
On 2012/06/14 09:09:28, Yang wrote:
"to evaluate"?
Done.
http://codereview.chromium.org/10544151/diff/1/src/liveedit-debugger.js
File src/liveedit-debugger.js (right):
http://codereview.chromium.org/10544151/diff/1/src/liveedit-debugger.js#newcode50
src/liveedit-debugger.js:50: const NEEDS_STEP_IN_PROPERTY_NAME =
"stack_update_needs_step_in";
On 2012/06/14 09:09:28, Yang wrote:
Do not use const in native javascript.
Done.
http://codereview.chromium.org/10544151/diff/1/src/liveedit.cc
File src/liveedit.cc (right):
http://codereview.chromium.org/10544151/diff/1/src/liveedit.cc#newcode1603
src/liveedit.cc:1603: MultipleFunctionTarget(Handle<JSArray>
shared_info_array, Handle<JSArray> result)
On 2012/06/14 09:09:28, Yang wrote:
80 char limit.
Done.
http://codereview.chromium.org/10544151/diff/1/src/liveedit.cc#newcode1607
src/liveedit.cc:1607: }
On 2012/06/14 09:09:28, Yang wrote:
You can put the empty function body on the same line as the last item
in the
initialization list.
Done.
http://codereview.chromium.org/10544151/diff/1/src/liveedit.cc#newcode1608
src/liveedit.cc:1608: bool MatchActivation(StackFrame* frame,
LiveEdit::FunctionPatchabilityStatus status) {
On 2012/06/14 09:09:28, Yang wrote:
80 char limit.
Done.
http://codereview.chromium.org/10544151/diff/1/src/liveedit.cc#newcode1638
src/liveedit.cc:1638: frame,
LiveEdit::FUNCTION_BLOCKED_UNDER_NATIVE_CODE)) {
On 2012/06/14 09:09:28, Yang wrote:
4 spaces more indent.
Done.
http://codereview.chromium.org/10544151/diff/1/src/liveedit.cc#newcode1661
src/liveedit.cc:1661: frame,
LiveEdit::FUNCTION_BLOCKED_ON_ACTIVE_STACK)) {
On 2012/06/14 09:09:28, Yang wrote:
ditto.
Done.
http://codereview.chromium.org/10544151/diff/1/src/liveedit.cc#newcode1674
src/liveedit.cc:1674: frame,
LiveEdit::FUNCTION_BLOCKED_UNDER_NATIVE_CODE)) {
On 2012/06/14 09:09:28, Yang wrote:
ditto.
Done.
http://codereview.chromium.org/10544151/diff/1/src/liveedit.cc#newcode1722
src/liveedit.cc:1722: const char* message =
DropActivationsInActiveThreadImpl(target, do_drop, zone);
On 2012/06/14 09:09:28, Yang wrote:
80 char limit.
Done.
http://codereview.chromium.org/10544151/diff/1/src/liveedit.cc#newcode1810
src/liveedit.cc:1810: bool MatchActivation(StackFrame* frame,
LiveEdit::FunctionPatchabilityStatus status) {
On 2012/06/14 09:09:28, Yang wrote:
80 char limit.
Done.
http://codereview.chromium.org/10544151/diff/1/src/mirror-debugger.js
File src/mirror-debugger.js (right):
http://codereview.chromium.org/10544151/diff/1/src/mirror-debugger.js#newcode1755
src/mirror-debugger.js:1755: if (result === void 0) {
On 2012/06/14 09:09:28, Yang wrote:
I think using the IS_UNDEFINED macro is nicer.
Done.
http://codereview.chromium.org/10544151/diff/1/src/runtime.cc
File src/runtime.cc (right):
http://codereview.chromium.org/10544151/diff/1/src/runtime.cc#newcode12796
src/runtime.cc:12796: // Returns 'true' if success or undefined or error
message elseway.
On 2012/06/14 09:09:28, Yang wrote:
Ambiguous sentence. How about "Returns true if successful. Otherwise
returns
undefined or an error message."
Perfect fit for 80 columns!
Done
http://codereview.chromium.org/10544151/diff/1/src/runtime.cc#newcode12825
src/runtime.cc:12825: const char* error_message =
LiveEdit::RestartFrame(it.frame(), isolate->zone());
On 2012/06/14 09:09:28, Yang wrote:
80 character limit.
Done.
http://codereview.chromium.org/10544151/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
