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

Issue 1342543004: Consolidate constructormagic macros with Chromium version and remove Chromium override. (Closed)

Created:
5 years, 3 months ago by Henrik Grunell WebRTC
Modified:
5 years, 2 months ago
Reviewers:
tommi, Andrew MacDonald
CC:
webrtc-reviews_webrtc.org, yujie_mao (webrtc), Andrew MacDonald, stefan-webrtc, tterriberry_mozilla.com, qiang.lu, niklas.enbom, mflodman
Base URL:
https://chromium.googlesource.com/external/webrtc.git@master
Target Ref:
refs/pending/heads/master
Project:
webrtc
Visibility:
Public.

Description

Consolidate constructormagic macros with Chromium version and remove Chromium override. Part of work removing dependency on Chromium's base. Only adds "= delete". From https://codereview.chromium.org/1151443003 : "This will guarantee the error to be at compile time, and not rely on the call visibility (private)." In consequence of that change, fixed an illegal copy and removed a bunch of unused variables. Depends on https://codereview.webrtc.org/1345433002/ BUG=chromium:468375 (in particular comment #37) NOTRY=true Committed: https://crrev.com/384194369b4be41912353631a68689129a49e58c Cr-Commit-Position: refs/heads/master@{#9954}

Patch Set 1 : Original CL that had to be reverted. #

Patch Set 2 : Rebase on rename macro CL. #

Patch Set 3 : Rebase. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+16 lines, -46 lines) Patch
M talk/app/webrtc/videosource.cc View 1 1 chunk +0 lines, -2 lines 0 comments Download
M talk/media/base/streamparams.h View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M webrtc/base/constructormagic.h View 1 2 1 chunk +11 lines, -10 lines 0 comments Download
M webrtc/base/logging.cc View 1 2 1 chunk +1 line, -2 lines 0 comments Download
M webrtc/base/natserver.h View 1 1 chunk +0 lines, -1 line 0 comments Download
M webrtc/base/socket.h View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M webrtc/base/virtualsocketserver.h View 1 1 chunk +0 lines, -1 line 0 comments Download
M webrtc/base/virtualsocketserver.cc View 1 chunk +0 lines, -1 line 0 comments Download
M webrtc/modules/remote_bitrate_estimator/test/bwe_test_framework.h View 1 2 chunks +0 lines, -3 lines 0 comments Download
M webrtc/modules/remote_bitrate_estimator/test/bwe_test_framework.cc View 1 1 chunk +1 line, -2 lines 0 comments Download
M webrtc/modules/rtp_rtcp/source/rtp_format_h264.h View 1 1 chunk +0 lines, -1 line 0 comments Download
M webrtc/modules/rtp_rtcp/source/rtp_format_h264.cc View 1 chunk +1 line, -2 lines 0 comments Download
M webrtc/modules/video_coding/main/test/test_util.h View 1 1 chunk +0 lines, -1 line 0 comments Download
D webrtc/overrides/webrtc/base/constructormagic.h View 1 1 chunk +0 lines, -20 lines 0 comments Download

Depends on Patchset:

Messages

Total messages: 17 (5 generated)
Henrik Grunell WebRTC
5 years, 3 months ago (2015-09-14 12:25:27 UTC) #2
tommi
lgtm
5 years, 3 months ago (2015-09-14 14:11:33 UTC) #3
Andrew MacDonald
Code looks good, but we should wait on this until your mailing list thread arrives ...
5 years, 3 months ago (2015-09-14 16:19:42 UTC) #4
Henrik Grunell WebRTC
On 2015/09/14 16:19:42, Andrew MacDonald wrote: > Code looks good, but we should wait on ...
5 years, 3 months ago (2015-09-14 19:25:24 UTC) #5
Andrew MacDonald
lgtm assuming no further concerns are brought up on your email thread.
5 years, 3 months ago (2015-09-15 01:51:44 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1342543004/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1342543004/40001
5 years, 3 months ago (2015-09-16 12:40:47 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1342543004/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1342543004/40001
5 years, 3 months ago (2015-09-16 13:32:00 UTC) #12
commit-bot: I haz the power
Committed patchset #3 (id:40001)
5 years, 3 months ago (2015-09-16 13:33:25 UTC) #13
commit-bot: I haz the power
Patchset 3 (id:??) landed as https://crrev.com/384194369b4be41912353631a68689129a49e58c Cr-Commit-Position: refs/heads/master@{#9954}
5 years, 3 months ago (2015-09-16 13:33:29 UTC) #14
Costent
On 2015/09/16 13:33:29, commit-bot: I haz the power wrote: > Patchset 3 (id:??) landed as ...
5 years, 2 months ago (2015-10-14 13:09:06 UTC) #15
Henrik Grunell WebRTC
On 2015/10/14 13:09:06, Costent wrote: > On 2015/09/16 13:33:29, commit-bot: I haz the power wrote: ...
5 years, 2 months ago (2015-10-14 13:48:04 UTC) #16
Costent
5 years, 2 months ago (2015-10-18 12:05:38 UTC) #17
Message was sent while issue was closed.
On 2015/10/14 13:48:04, Henrik Grunell (webrtc) wrote:
> On 2015/10/14 13:09:06, Costent wrote:
> > On 2015/09/16 13:33:29, commit-bot: I haz the power wrote:
> > > Patchset 3 (id:??) landed as
> > > https://crrev.com/384194369b4be41912353631a68689129a49e58c
> > > Cr-Commit-Position: refs/heads/master@{#9954}
> > 
> > `webrtc/overrides/webrtc/base/constructormagic.h` is still referenced in
> > `webrtc/base/base.gyp`.
> > See: https://codereview.webrtc.org/1394223003/
> 
> I don't you have synced the latest code.
> See
>
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/webrtc...

Indeed! My bad.

Powered by Google App Engine
This is Rietveld 408576698