Closed Bug 1217692 Opened 4 years ago Closed 4 years ago

Fix some styles of MediaDecoderReader.h

Categories

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

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla44
Tracking Status
firefox44 --- fixed

People

(Reporter: jwwang, Assigned: jwwang)

Details

Attachments

(2 files)

No description provided.
Assignee: nobody → jwwang
Bug 1217692. Part 1 - move members that don't have to be public to protected or private sections. r=jya.
Attachment #8677964 - Flags: review?(jyavenard)
Bug 1217692. Part 2 - fix some styles to keep 80 cols limit. r=jya.
Attachment #8677965 - Flags: review?(jyavenard)
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.
Attachment #8677964 - Flags: review?(jyavenard) → review+
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.
Attachment #8677965 - Flags: review?(jyavenard)
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
Attachment #8677965 - Flags: review+
Thanks for the review!
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.
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.
https://hg.mozilla.org/mozilla-central/rev/64cbf3e1fe27
https://hg.mozilla.org/mozilla-central/rev/2f1393553a48
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
You need to log in before you can comment on or make changes to this bug.