modules/rtp_rtcp/include folder cleared of lint warnings

modules/rtp_rtcp/include folder cleared of lint warnings
Functions that do not follow lint are marked deprecated, including function in the interface.

BUG=webrtc:5308
R=mflodman@webrtc.org

Committed: https://crrev.com/5c1def8892390a336d1eecd9b61adacece858898
Cr-Commit-Position: refs/heads/master@{#10975}

[danilchap, 2015-12-04 14:07:43]

Description was changed from

==========
modules/rtp_rtcp/include folder cleared of lint warnings
Functions that do not follow lint are marked depricated, including function in
the interface.

BUG=webrtc:5308
R=mflodman@webrtc.org
CC=pbos@webrtc.org
==========

to

==========
modules/rtp_rtcp/include folder cleared of lint warnings
Functions that do not follow lint are marked depricated, including function in
the interface.

BUG=webrtc:5308
R=mflodman@webrtc.org
==========

[danilchap, 2015-12-04 14:11:22]

putting on review (though only one small folder is cleaned) to check if I
understand goal correctly.

[pbos-webrtc, 2015-12-04 14:14:09]

On 2015/12/04 14:11:22, danilchap wrote:
> putting on review (though only one small folder is cleaned) to check if I
> understand goal correctly.

The idea is that you should fix the warnings not make the lint warnings be
ignored (so this includes changing non-const references to be pointers instead)

[danilchap, 2015-12-04 14:43:22]

On 2015/12/04 14:14:09, pbos-webrtc wrote:
> On 2015/12/04 14:11:22, danilchap wrote:
> > putting on review (though only one small folder is cleaned) to check if I
> > understand goal correctly.
> 
> The idea is that you should fix the warnings not make the lint warnings be
> ignored (so this includes changing non-const references to be pointers
instead)

Yes, that what I understand. Unfortunately that conflicts with idea 'do not
change interface immediately'.
So I leave old interface, mark it Depricated and NOLINT. And create nearby
'correct' interface, the one LINT do not complain about.
I expect this complications wouldn't be necessary for /source folder.

[mflodman, 2015-12-09 09:19:13]

A few minor comments.

https://codereview.webrtc.org/1493403003/diff/20001/webrtc/modules/rtp_rtcp/i...
File webrtc/modules/rtp_rtcp/include/rtp_payload_registry.h (right):

https://codereview.webrtc.org/1493403003/diff/20001/webrtc/modules/rtp_rtcp/i...
webrtc/modules/rtp_rtcp/include/rtp_payload_registry.h:113: // Depricated.
Here and elsewhere:
s/depricated/deprecated

https://codereview.webrtc.org/1493403003/diff/20001/webrtc/modules/rtp_rtcp/i...
File webrtc/modules/rtp_rtcp/include/rtp_rtcp.h (right):

https://codereview.webrtc.org/1493403003/diff/20001/webrtc/modules/rtp_rtcp/i...
webrtc/modules/rtp_rtcp/include/rtp_rtcp.h:16: #include <utility>
Alphabetic order.

https://codereview.webrtc.org/1493403003/diff/20001/webrtc/modules/rtp_rtcp/m...
File webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h (right):

https://codereview.webrtc.org/1493403003/diff/20001/webrtc/modules/rtp_rtcp/m...
webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h:230:
MOCK_METHOD3(GenericFECStatus,
I think you should keep the old method too until it's removed from the
interface.

https://codereview.webrtc.org/1493403003/diff/20001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtp_payload_registry.cc (right):

https://codereview.webrtc.org/1493403003/diff/20001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtp_payload_registry.cc:347: if (payload ==
nullptr) {
We've started going towards 'if (!payload)' rather than 'if (paayload ==
nullptr)'.

https://codereview.webrtc.org/1493403003/diff/20001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtp_receiver_impl.cc (right):

https://codereview.webrtc.org/1493403003/diff/20001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtp_receiver_impl.cc:281: if (payload == nullptr)
{
Same for the check here and below.

[danilchap, 2015-12-09 11:22:36]

Description was changed from

==========
modules/rtp_rtcp/include folder cleared of lint warnings
Functions that do not follow lint are marked depricated, including function in
the interface.

BUG=webrtc:5308
R=mflodman@webrtc.org
==========

to

==========
modules/rtp_rtcp/include folder cleared of lint warnings
Functions that do not follow lint are marked deprecated, including function in
the interface.

BUG=webrtc:5308
R=mflodman@webrtc.org
==========

[danilchap, 2015-12-09 11:23:37]

https://codereview.webrtc.org/1493403003/diff/20001/webrtc/modules/rtp_rtcp/i...
File webrtc/modules/rtp_rtcp/include/rtp_payload_registry.h (right):

https://codereview.webrtc.org/1493403003/diff/20001/webrtc/modules/rtp_rtcp/i...
webrtc/modules/rtp_rtcp/include/rtp_payload_registry.h:113: // Depricated.
On 2015/12/09 09:19:13, mflodman wrote:
> Here and elsewhere:
> s/depricated/deprecated

Done.

https://codereview.webrtc.org/1493403003/diff/20001/webrtc/modules/rtp_rtcp/i...
File webrtc/modules/rtp_rtcp/include/rtp_rtcp.h (right):

https://codereview.webrtc.org/1493403003/diff/20001/webrtc/modules/rtp_rtcp/i...
webrtc/modules/rtp_rtcp/include/rtp_rtcp.h:16: #include <utility>
On 2015/12/09 09:19:13, mflodman wrote:
> Alphabetic order.

Done.

https://codereview.webrtc.org/1493403003/diff/20001/webrtc/modules/rtp_rtcp/m...
File webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h (right):

https://codereview.webrtc.org/1493403003/diff/20001/webrtc/modules/rtp_rtcp/m...
webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h:230:
MOCK_METHOD3(GenericFECStatus,
On 2015/12/09 09:19:13, mflodman wrote:
> I think you should keep the old method too until it's removed from the
> interface.

GMock documentation tells that virtual functions should be mocked.
Old method became a non-virtual inline that will automatically call new method.

https://codereview.webrtc.org/1493403003/diff/20001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtp_payload_registry.cc (right):

https://codereview.webrtc.org/1493403003/diff/20001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtp_payload_registry.cc:347: if (payload ==
nullptr) {
On 2015/12/09 09:19:13, mflodman wrote:
> We've started going towards 'if (!payload)' rather than 'if (paayload ==
> nullptr)'.

Done.

https://codereview.webrtc.org/1493403003/diff/20001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtp_receiver_impl.cc (right):

https://codereview.webrtc.org/1493403003/diff/20001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtp_receiver_impl.cc:281: if (payload == nullptr)
{
On 2015/12/09 09:19:13, mflodman wrote:
> Same for the check here and below.

Done.

[mflodman, 2015-12-10 08:24:23]

One comment, then LGTM.

https://codereview.webrtc.org/1493403003/diff/20001/webrtc/modules/rtp_rtcp/m...
File webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h (right):

https://codereview.webrtc.org/1493403003/diff/20001/webrtc/modules/rtp_rtcp/m...
webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h:230:
MOCK_METHOD3(GenericFECStatus,
On 2015/12/09 11:23:36, danilchap wrote:
> On 2015/12/09 09:19:13, mflodman wrote:
> > I think you should keep the old method too until it's removed from the
> > interface.
> 
> GMock documentation tells that virtual functions should be mocked.
> Old method became a non-virtual inline that will automatically call new
method.

Acknowledged.

https://codereview.webrtc.org/1493403003/diff/40001/webrtc/modules/rtp_rtcp/i...
File webrtc/modules/rtp_rtcp/include/rtp_payload_registry.h (right):

https://codereview.webrtc.org/1493403003/diff/40001/webrtc/modules/rtp_rtcp/i...
webrtc/modules/rtp_rtcp/include/rtp_payload_registry.h:120: return payload;
Here I'd still prefer to return != nullptr to clerly show it's a bool, but let's
discuss if you disagree.

[danilchap, 2015-12-10 09:01:15]

https://codereview.webrtc.org/1493403003/diff/40001/webrtc/modules/rtp_rtcp/i...
File webrtc/modules/rtp_rtcp/include/rtp_payload_registry.h (right):

https://codereview.webrtc.org/1493403003/diff/40001/webrtc/modules/rtp_rtcp/i...
webrtc/modules/rtp_rtcp/include/rtp_payload_registry.h:120: return payload;
On 2015/12/10 08:24:23, mflodman wrote:
> Here I'd still prefer to return != nullptr to clerly show it's a bool, but
let's
> discuss if you disagree.

I have arguments both for and against it, but both of them are week for a small
deprecated function.
So would go for payload != nullptr since you and my taste agree this for is more
clear.

[danilchap, 2015-12-10 13:45:34]

The CQ bit was checked by danilchap@webrtc.org

[danilchap, 2015-12-10 13:45:35]

The patchset sent to the CQ was uploaded after l-g-t-m from mflodman@webrtc.org
Link to the patchset: https://codereview.webrtc.org/1493403003/#ps80001 (title:
"rebase")

[commit-bot: I haz the power, 2015-12-10 13:45:45]

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

[commit-bot: I haz the power, 2015-12-10 15:46:02]

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

[commit-bot: I haz the power, 2015-12-10 15:46:03]

Try jobs failed on following builders:
  android_arm64_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  linux_baremetal on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  mac_baremetal on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  win_baremetal on tryserver.webrtc (JOB_TIMED_OUT, no build URL)

[danilchap, 2015-12-10 17:48:15]

The CQ bit was checked by danilchap@webrtc.org

[commit-bot: I haz the power, 2015-12-10 17:48:24]

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

[commit-bot: I haz the power, 2015-12-10 17:51:59]

Description was changed from

==========
modules/rtp_rtcp/include folder cleared of lint warnings
Functions that do not follow lint are marked deprecated, including function in
the interface.

BUG=webrtc:5308
R=mflodman@webrtc.org
==========

to

==========
modules/rtp_rtcp/include folder cleared of lint warnings
Functions that do not follow lint are marked deprecated, including function in
the interface.

BUG=webrtc:5308
R=mflodman@webrtc.org
==========

[commit-bot: I haz the power, 2015-12-10 17:52:00]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-12-10 17:52:07]

Description was changed from

==========
modules/rtp_rtcp/include folder cleared of lint warnings
Functions that do not follow lint are marked deprecated, including function in
the interface.

BUG=webrtc:5308
R=mflodman@webrtc.org
==========

to

==========
modules/rtp_rtcp/include folder cleared of lint warnings
Functions that do not follow lint are marked deprecated, including function in
the interface.

BUG=webrtc:5308
R=mflodman@webrtc.org

Committed: https://crrev.com/5c1def8892390a336d1eecd9b61adacece858898
Cr-Commit-Position: refs/heads/master@{#10975}
==========

[commit-bot: I haz the power, 2015-12-10 17:52:08]

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