Closed Bug 1395296 Opened 3 years ago Closed 1 year ago

Add /media/mtransport/ to the list of ignore

Categories

(Firefox Build System :: Source Code Analysis, enhancement)

enhancement
Not set

Tracking

(firefox57 affected)

RESOLVED WONTFIX
Tracking Status
firefox57 --- affected

People

(Reporter: Sylvestre, Assigned: Sylvestre)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

ekr just told me that it is used the Google coding style
Attached patch mtransport.diff (obsolete) — Splinter Review
Attachment #8902867 - Flags: review?(ekr)
Comment on attachment 8902867 [details] [diff] [review]
mtransport.diff

Review of attachment 8902867 [details] [diff] [review]:
-----------------------------------------------------------------

I think you can also remove media/mtransport/test (because it is duplicated)

::: tools/rewriting/ThirdPartyPaths.txt
@@ +39,5 @@
>  media/libtremor/
>  media/libvorbis/
>  media/libvpx/
>  media/libyuv/
> +media/mtransport/

This doesn't seem right. We wrote media/mtransport/[other than third party], we just used Google style
> we just used Google style
Is it because we share this code with Google or just because the developers preferred Google's style over Mozilla's?
Attached patch mtransport.diffSplinter Review
Attachment #8902867 - Attachment is obsolete: true
Attachment #8902867 - Flags: review?(ekr)
Attachment #8902875 - Flags: review?(ekr)
(In reply to Sylvestre Ledru [:sylvestre] from comment #3)
> > we just used Google style
> Is it because we share this code with Google or just because the developers
> preferred Google's style over Mozilla's?

The latter
Comment on attachment 8902875 [details] [diff] [review]
mtransport.diff

Review of attachment 8902875 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM
Attachment #8902875 - Flags: review?(ekr) → review+
Product: Core → Firefox Build System
Did you want to land this?
Flags: needinfo?(sledru)
Not yet, we are still having discussions about coding style.
Flags: needinfo?(sledru)
Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.