1217692 - Fix some styles of MediaDecoderReader.h

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect)

Comment 1

[JW Wang [:jwwang] [:jw_wang]]

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a7e90eafa058

Comment 2

[JW Wang [:jwwang] [:jw_wang]]

Attachment: MozReview Request: Bug 1217692. Part 1 - move members that don't have to be public to protected or private sections. r=jya.

Bug 1217692. Part 1 - move members that don't have to be public to protected or private sections. r=jya.

Comment 3

[JW Wang [:jwwang] [:jw_wang]]

Attachment: MozReview Request: Bug 1217692. Part 2 - fix some styles to keep 80 cols limit. r=jya.

Bug 1217692. Part 2 - fix some styles to keep 80 cols limit. r=jya.

Comment 4

[Jean-Yves Avenard [:jya]]

Comment on attachment 8677964 [details]
MozReview Request: Bug 1217692. Part 1 - move members that don't have to be public to protected or private sections. r=jya.

https://reviewboard.mozilla.org/r/23065/#review20559

::: dom/media/MediaDecoderReader.h:64
(Diff revision 1)
> +  friend class TrackBuffer;

I typically have the friend declaration above the members being accessed ; makes it easier to track why they need to be friends.

The TrackBuffer class no longer exists.

Comment 5

[Jean-Yves Avenard [:jya]]

Comment on attachment 8677965 [details]
MozReview Request: Bug 1217692. Part 2 - fix some styles to keep 80 cols limit. r=jya.

https://reviewboard.mozilla.org/r/23067/#review20561

Shouldn't we have the return type on a line above the function name?
e.g.:

void
MediaDecoderReader::Blah(...)
{
..
}

this is seems to be the most common type used.

Comment 6

[Jean-Yves Avenard [:jya]]

Comment on attachment 8677965 [details]
MozReview Request: Bug 1217692. Part 2 - fix some styles to keep 80 cols limit. r=jya.

https://reviewboard.mozilla.org/r/23067/#review20563

Comment 7

[JW Wang [:jwwang] [:jw_wang]]

Thanks for the review!

Comment 8

[JW Wang [:jwwang] [:jw_wang]]

https://reviewboard.mozilla.org/r/23067/#review20561

Which line do you mean? I saw the style is used in .cpp files only. Not sure if it applies to .h as well.

Comment 9

[JW Wang [:jwwang] [:jw_wang]]

https://reviewboard.mozilla.org/r/23065/#review20559

> I typically have the friend declaration above the members being accessed ; makes it easier to track why they need to be friends.
> 
> The TrackBuffer class no longer exists.

If the friend only accesses only one member or two, it would be nice to put related things together. But sometimes a friend access several members and it would be a bit messy to put them together.

Comment 10

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/64cbf3e1fe27
https://hg.mozilla.org/integration/mozilla-inbound/rev/2f1393553a48

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/64cbf3e1fe27
https://hg.mozilla.org/mozilla-central/rev/2f1393553a48