Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(38)

Issue 2832263004: Fixing some strings that go into webrtc-internals. (Closed)

Created:
2 years, 7 months ago by Taylor_Brandstetter
Modified:
2 years, 6 months ago
CC:
chromium-reviews, posciak+watch_chromium.org, jam, darin-cc_chromium.org, mlamouri+watch-content_chromium.org, feature-media-reviews_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Fixing some strings that go into webrtc-internals. Mainly, some enums were being displayed in their raw form, due to the Blink Rename. For example, "kSignalingStateStable" instead of "stable". BUG=chromium:714375 Review-Url: https://codereview.chromium.org/2832263004 Cr-Commit-Position: refs/heads/master@{#467267} Committed: https://chromium.googlesource.com/chromium/src/+/fb4604d257703df68c8865db1a5d27f3059c2920

Patch Set 1 #

Total comments: 2

Patch Set 2 : Use different enum names between C++ and JS. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+28 lines, -25 lines) Patch
M content/renderer/media/peer_connection_tracker.cc View 1 3 chunks +28 lines, -25 lines 0 comments Download

Messages

Total messages: 17 (7 generated)
Taylor_Brandstetter
PTAL
2 years, 7 months ago (2017-04-22 11:46:54 UTC) #2
tommi (sloooow) - chröme
https://codereview.chromium.org/2832263004/diff/1/content/renderer/media/peer_connection_tracker.cc File content/renderer/media/peer_connection_tracker.cc (right): https://codereview.chromium.org/2832263004/diff/1/content/renderer/media/peer_connection_tracker.cc#newcode188 content/renderer/media/peer_connection_tracker.cc:188: #define GET_STRING_OF_STATE(state) \ what about fixing the macro instead? ...
2 years, 7 months ago (2017-04-24 11:07:49 UTC) #3
Taylor_Brandstetter
https://codereview.chromium.org/2832263004/diff/1/content/renderer/media/peer_connection_tracker.cc File content/renderer/media/peer_connection_tracker.cc (right): https://codereview.chromium.org/2832263004/diff/1/content/renderer/media/peer_connection_tracker.cc#newcode188 content/renderer/media/peer_connection_tracker.cc:188: #define GET_STRING_OF_STATE(state) \ On 2017/04/24 11:07:48, tommi - chröme ...
2 years, 6 months ago (2017-04-24 20:24:41 UTC) #4
tommi
lgtm Thanks, yes it's a bit convoluted but I think it might be a good ...
2 years, 6 months ago (2017-04-25 08:34:59 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2832263004/20001
2 years, 6 months ago (2017-04-25 08:35:11 UTC) #8
commit-bot: I haz the power
No L-G-T-M from a valid reviewer yet. CQ run can only be started once the ...
2 years, 6 months ago (2017-04-25 08:35:12 UTC) #10
Taylor_Brandstetter
Need an lgtm from tommi chröme
2 years, 6 months ago (2017-04-25 18:46:11 UTC) #11
tommi (sloooow) - chröme
Oops, lgtm
2 years, 6 months ago (2017-04-26 07:26:53 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2832263004/20001
2 years, 6 months ago (2017-04-26 07:27:40 UTC) #14
commit-bot: I haz the power
2 years, 6 months ago (2017-04-26 08:27:01 UTC) #17
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/fb4604d257703df68c8865db1a5d...

Powered by Google App Engine
This is Rietveld 408576698