🐜 GN: Make write_runtime_deps take effect only a target is resolved

GN: Make write_runtime_deps take effect only a target is resolved

BUG=604104
Committed: https://crrev.com/fb8986d4df3e8b84db16cd5d719a2e38bb8a7b02
Cr-Commit-Position: refs/heads/master@{#388496}

[agrieve, 2016-04-16 01:40:21]

agrieve@chromium.org changed reviewers:
+ brettw@chromium.org

[agrieve, 2016-04-19 18:16:34]

On 2016/04/16 01:40:22, agrieve wrote:

🐥

[brettw, 2016-04-19 18:22:37]

👌

[brettw, 2016-04-19 18:29:39]

lgtm

https://codereview.chromium.org/1892393002/diff/1/tools/gn/target.cc
File tools/gn/target.cc (right):

https://codereview.chromium.org/1892393002/diff/1/tools/gn/target.cc#newcode327
tools/gn/target.cc:327: if (!write_runtime_deps_output_.value().empty()) {
No {}

https://codereview.chromium.org/1892393002/diff/1/tools/gn/test_with_scope.cc
File tools/gn/test_with_scope.cc (right):

https://codereview.chromium.org/1892393002/diff/1/tools/gn/test_with_scope.cc...
tools/gn/test_with_scope.cc:44: if (err != nullptr) {
I've not used optional errors in any places to avoid this type of checking
getting out of control or being inconsistent. Can you remove the null checks for
Err for this function? (And you can clean up my_err below).

[agrieve, 2016-04-20 00:45:03]

https://codereview.chromium.org/1892393002/diff/1/tools/gn/target.cc
File tools/gn/target.cc (right):

https://codereview.chromium.org/1892393002/diff/1/tools/gn/target.cc#newcode327
tools/gn/target.cc:327: if (!write_runtime_deps_output_.value().empty()) {
On 2016/04/19 18:29:39, brettw wrote:
> No {}

Done. (a hard habit to break apparently!)

https://codereview.chromium.org/1892393002/diff/1/tools/gn/test_with_scope.cc
File tools/gn/test_with_scope.cc (right):

https://codereview.chromium.org/1892393002/diff/1/tools/gn/test_with_scope.cc...
tools/gn/test_with_scope.cc:44: if (err != nullptr) {
On 2016/04/19 18:29:39, brettw wrote:
> I've not used optional errors in any places to avoid this type of checking
> getting out of control or being inconsistent. Can you remove the null checks
for
> Err for this function? (And you can clean up my_err below).

Done.

[agrieve, 2016-04-20 00:45:17]

The CQ bit was checked by agrieve@chromium.org

[agrieve, 2016-04-20 00:45:17]

The patchset sent to the CQ was uploaded after l-g-t-m from brettw@chromium.org
Link to the patchset: https://codereview.chromium.org/1892393002/#ps20001
(title: "review comments")

[commit-bot: I haz the power, 2016-04-20 00:45:40]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1892393002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1892393002/20001

[commit-bot: I haz the power, 2016-04-20 01:02:40]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-04-20 01:02:41]

Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[agrieve, 2016-04-20 14:06:27]

The CQ bit was checked by agrieve@chromium.org

[agrieve, 2016-04-20 14:06:28]

The patchset sent to the CQ was uploaded after l-g-t-m from brettw@chromium.org
Link to the patchset: https://codereview.chromium.org/1892393002/#ps40001
(title: "Fix test")

[commit-bot: I haz the power, 2016-04-20 14:06:41]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1892393002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1892393002/40001

[commit-bot: I haz the power, 2016-04-20 14:50:04]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-04-22 19:23:24]

Description was changed from

==========
GN: Make write_runtime_deps take effect only a target is resolved

BUG=604104
==========

to

==========
GN: Make write_runtime_deps take effect only a target is resolved

BUG=604104

Committed: https://crrev.com/fb8986d4df3e8b84db16cd5d719a2e38bb8a7b02
Cr-Commit-Position: refs/heads/master@{#388496}
==========

[commit-bot: I haz the power, 2016-04-22 19:23:25]

Patchset 3 (id:??) landed as
https://crrev.com/fb8986d4df3e8b84db16cd5d719a2e38bb8a7b02
Cr-Commit-Position: refs/heads/master@{#388496}