Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(48)

Issue 149253013: Document ProcessSignal::watch. (Closed)

Created:
6 years, 10 months ago by Anders Johnsen
Modified:
6 years, 10 months ago
CC:
reviews_dartlang.org, vm-dev_dartlang.org
Visibility:
Public.

Description

Document ProcessSignal::watch. We now also allow watching for SIGTERM. BUG= R=sgjesse@google.com Committed: https://code.google.com/p/dart/source/detail?r=32416

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+27 lines, -9 lines) Patch
M runtime/bin/process_android.cc View 1 chunk +4 lines, -3 lines 0 comments Download
M runtime/bin/process_linux.cc View 1 chunk +4 lines, -3 lines 0 comments Download
M runtime/bin/process_macos.cc View 1 chunk +4 lines, -3 lines 0 comments Download
M runtime/bin/process_patch.dart View 1 chunk +1 line, -0 lines 2 comments Download
M sdk/lib/io/process.dart View 1 chunk +14 lines, -0 lines 0 comments Download

Messages

Total messages: 5 (0 generated)
Anders Johnsen
6 years, 10 months ago (2014-02-07 08:49:54 UTC) #1
Søren Gjesse
lgtm
6 years, 10 months ago (2014-02-07 09:28:54 UTC) #2
Anders Johnsen
Committed patchset #1 manually as r32416 (presubmit successful).
6 years, 10 months ago (2014-02-07 09:41:47 UTC) #3
Bob Nystrom
https://codereview.chromium.org/149253013/diff/1/runtime/bin/process_patch.dart File runtime/bin/process_patch.dart (right): https://codereview.chromium.org/149253013/diff/1/runtime/bin/process_patch.dart#newcode133 runtime/bin/process_patch.dart:133: signal != ProcessSignal.SIGTERM && Drive-by comment: This looks like ...
6 years, 10 months ago (2014-02-07 18:20:52 UTC) #4
Anders Johnsen
6 years, 10 months ago (2014-02-13 10:43:33 UTC) #5
Message was sent while issue was closed.
https://codereview.chromium.org/149253013/diff/1/runtime/bin/process_patch.dart
File runtime/bin/process_patch.dart (right):

https://codereview.chromium.org/149253013/diff/1/runtime/bin/process_patch.da...
runtime/bin/process_patch.dart:133: signal != ProcessSignal.SIGTERM &&
On 2014/02/07 18:20:52, Bob Nystrom wrote:
> Drive-by comment:
> 
> This looks like it's adding support for something new, but I don't see any
tests
> in this patch. That means to me that there is effectively an untested feature
> here. If this is something dart:io officially supports, shouldn't it be
tested?

Partly, signals are indeed tested, but not SIGTERM specifically. Added test.

Powered by Google App Engine
This is Rietveld 408576698