Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2343813003:
Add voe_cmd_test to voice_engine/BUILD.gn (Closed)
|
Created:
4 years, 3 months ago by the sun Modified:
4 years, 3 months ago Reviewers:
kjellander_webrtc CC:
webrtc-reviews_webrtc.org, henrika_webrtc, Andrew MacDonald, tterriberry_mozilla.com, audio-team_agora.io, hlundin-webrtc, peah-webrtc, minyue-webrtc Target Ref:
refs/pending/heads/master Project:
webrtc Visibility:
Public. |
DescriptionAdd voe_cmd_test to voice_engine/BUILD.gn (and remove it from voice_engine.gyp, together with the channel_transport gyp target)
BUG=webrtc:6323
NOTRY=True
Committed: https://crrev.com/fb2c1d0636262f00a366dda1e8e15e97eef4f242
Cr-Commit-Position: refs/heads/master@{#14243}
Patch Set 1 #
Total comments: 6
Patch Set 2 : comments #Patch Set 3 : rebase #
Messages
Total messages: 14 (6 generated)
Description was changed from ========== Add voe_cmd_test to voice_engine/BUILD.gn BUG=webrtc:6323 ========== to ========== Add voe_cmd_test to voice_engine/BUILD.gn (and remove it from voice_engine.gyp, together with the channel_transport gyp target) BUG=webrtc:6323 ==========
solenberg@webrtc.org changed reviewers: + kjellander@webrtc.org
lgtm with nits and a question. Don't forget NOTRY=True https://codereview.webrtc.org/2343813003/diff/1/webrtc/voice_engine/BUILD.gn File webrtc/voice_engine/BUILD.gn (right): https://codereview.webrtc.org/2343813003/diff/1/webrtc/voice_engine/BUILD.gn#... webrtc/voice_engine/BUILD.gn:247: "//webrtc/:rtc_event_log", Write this as //webrtc:rtc_event_log https://codereview.webrtc.org/2343813003/diff/1/webrtc/voice_engine/BUILD.gn#... webrtc/voice_engine/BUILD.gn:249: "//webrtc/system_wrappers/:system_wrappers_default", Same principle here and below. https://codereview.webrtc.org/2343813003/diff/1/webrtc/voice_engine/voice_eng... File webrtc/voice_engine/voice_engine.gyp (left): https://codereview.webrtc.org/2343813003/diff/1/webrtc/voice_engine/voice_eng... webrtc/voice_engine/voice_engine.gyp:114: 'defines': ['WEBRTC_DRIFT_COMPENSATION_SUPPORTED',], Did you mean to remove this with the tests? I'm confused what it actually affects since it doesn't sit inside a target.
https://codereview.webrtc.org/2343813003/diff/1/webrtc/voice_engine/BUILD.gn File webrtc/voice_engine/BUILD.gn (right): https://codereview.webrtc.org/2343813003/diff/1/webrtc/voice_engine/BUILD.gn#... webrtc/voice_engine/BUILD.gn:247: "//webrtc/:rtc_event_log", On 2016/09/15 14:05:31, kjellander_webrtc wrote: > Write this as //webrtc:rtc_event_log Done. https://codereview.webrtc.org/2343813003/diff/1/webrtc/voice_engine/BUILD.gn#... webrtc/voice_engine/BUILD.gn:249: "//webrtc/system_wrappers/:system_wrappers_default", On 2016/09/15 14:05:31, kjellander_webrtc wrote: > Same principle here and below. Done. https://codereview.webrtc.org/2343813003/diff/1/webrtc/voice_engine/voice_eng... File webrtc/voice_engine/voice_engine.gyp (left): https://codereview.webrtc.org/2343813003/diff/1/webrtc/voice_engine/voice_eng... webrtc/voice_engine/voice_engine.gyp:114: 'defines': ['WEBRTC_DRIFT_COMPENSATION_SUPPORTED',], On 2016/09/15 14:05:31, kjellander_webrtc wrote: > Did you mean to remove this with the tests? I'm confused what it actually > affects since it doesn't sit inside a target. It was intentional, but maybe I've misunderstood how things work - I thought it had no meaning by itself?
Description was changed from ========== Add voe_cmd_test to voice_engine/BUILD.gn (and remove it from voice_engine.gyp, together with the channel_transport gyp target) BUG=webrtc:6323 ========== to ========== Add voe_cmd_test to voice_engine/BUILD.gn (and remove it from voice_engine.gyp, together with the channel_transport gyp target) BUG=webrtc:6323 NOTRY=True ==========
On 2016/09/15 14:15:26, the sun wrote: > https://codereview.webrtc.org/2343813003/diff/1/webrtc/voice_engine/BUILD.gn > File webrtc/voice_engine/BUILD.gn (right): > > https://codereview.webrtc.org/2343813003/diff/1/webrtc/voice_engine/BUILD.gn#... > webrtc/voice_engine/BUILD.gn:247: "//webrtc/:rtc_event_log", > On 2016/09/15 14:05:31, kjellander_webrtc wrote: > > Write this as //webrtc:rtc_event_log > > Done. > > https://codereview.webrtc.org/2343813003/diff/1/webrtc/voice_engine/BUILD.gn#... > webrtc/voice_engine/BUILD.gn:249: > "//webrtc/system_wrappers/:system_wrappers_default", > On 2016/09/15 14:05:31, kjellander_webrtc wrote: > > Same principle here and below. > > Done. > > https://codereview.webrtc.org/2343813003/diff/1/webrtc/voice_engine/voice_eng... > File webrtc/voice_engine/voice_engine.gyp (left): > > https://codereview.webrtc.org/2343813003/diff/1/webrtc/voice_engine/voice_eng... > webrtc/voice_engine/voice_engine.gyp:114: 'defines': > ['WEBRTC_DRIFT_COMPENSATION_SUPPORTED',], > On 2016/09/15 14:05:31, kjellander_webrtc wrote: > > Did you mean to remove this with the tests? I'm confused what it actually > > affects since it doesn't sit inside a target. > > It was intentional, but maybe I've misunderstood how things work - I thought it > had no meaning by itself? Did you have anything to add Henrik, or can we assume it works if the bots don't spit it out?
On 2016/09/15 19:13:55, the sun wrote: > On 2016/09/15 14:15:26, the sun wrote: > > https://codereview.webrtc.org/2343813003/diff/1/webrtc/voice_engine/BUILD.gn > > File webrtc/voice_engine/BUILD.gn (right): > > > > > https://codereview.webrtc.org/2343813003/diff/1/webrtc/voice_engine/BUILD.gn#... > > webrtc/voice_engine/BUILD.gn:247: "//webrtc/:rtc_event_log", > > On 2016/09/15 14:05:31, kjellander_webrtc wrote: > > > Write this as //webrtc:rtc_event_log > > > > Done. > > > > > https://codereview.webrtc.org/2343813003/diff/1/webrtc/voice_engine/BUILD.gn#... > > webrtc/voice_engine/BUILD.gn:249: > > "//webrtc/system_wrappers/:system_wrappers_default", > > On 2016/09/15 14:05:31, kjellander_webrtc wrote: > > > Same principle here and below. > > > > Done. > > > > > https://codereview.webrtc.org/2343813003/diff/1/webrtc/voice_engine/voice_eng... > > File webrtc/voice_engine/voice_engine.gyp (left): > > > > > https://codereview.webrtc.org/2343813003/diff/1/webrtc/voice_engine/voice_eng... > > webrtc/voice_engine/voice_engine.gyp:114: 'defines': > > ['WEBRTC_DRIFT_COMPENSATION_SUPPORTED',], > > On 2016/09/15 14:05:31, kjellander_webrtc wrote: > > > Did you mean to remove this with the tests? I'm confused what it actually > > > affects since it doesn't sit inside a target. > > > > It was intentional, but maybe I've misunderstood how things work - I thought > it > > had no meaning by itself? > > Did you have anything to add Henrik, or can we assume it works if the bots don't > spit it out? No this is fine, I guess that define really did nothing (GYP probably just silently ignored it). lgtm
The CQ bit was checked by solenberg@webrtc.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
Message was sent while issue was closed.
Description was changed from ========== Add voe_cmd_test to voice_engine/BUILD.gn (and remove it from voice_engine.gyp, together with the channel_transport gyp target) BUG=webrtc:6323 NOTRY=True ========== to ========== Add voe_cmd_test to voice_engine/BUILD.gn (and remove it from voice_engine.gyp, together with the channel_transport gyp target) BUG=webrtc:6323 NOTRY=True ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== Add voe_cmd_test to voice_engine/BUILD.gn (and remove it from voice_engine.gyp, together with the channel_transport gyp target) BUG=webrtc:6323 NOTRY=True ========== to ========== Add voe_cmd_test to voice_engine/BUILD.gn (and remove it from voice_engine.gyp, together with the channel_transport gyp target) BUG=webrtc:6323 NOTRY=True Committed: https://crrev.com/fb2c1d0636262f00a366dda1e8e15e97eef4f242 Cr-Commit-Position: refs/heads/master@{#14243} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/fb2c1d0636262f00a366dda1e8e15e97eef4f242 Cr-Commit-Position: refs/heads/master@{#14243} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
