cdm-test-decryptor.cpp: Remove an unnecessary string initialization

RESOLVED FIXED in Firefox 60

Status

()

P3
normal
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: sylvestre, Assigned: vprabhu, Mentored)

Tracking

({good-first-bug})

Trunk
mozilla60
good-first-bug
Points:
---

Firefox Tracking Flags

(firefox60 fixed)

Details

(Whiteboard: [good first bug][lang=C++])

Attachments

(1 attachment)

(Reporter)

Description

a year ago
https://dxr.mozilla.org/mozilla-central/source/dom/media/fake-cdm/cdm-test-decryptor.cpp?q=cdm-test-decryptor.cpp&redirect_type=direct#429

Found by http://clang.llvm.org/extra/clang-tidy/checks/readability-redundant-string-init.html
(only occurrence left in our code base).

The checker will be enabled at review phase.

A trivial bug, marking it as a good first bug to learn about our workflows.

Comment 1

a year ago
I'd like to take this on as a first bug.

Comment 2

a year ago
I would like to work on this bug as my first task.
If you're interested in working on this bug, please assign it to yourself and post a patch for comments. Whoever takes it first can have a first go, but feel free to work together.
Priority: -- → P3

Comment 4

a year ago
Can anyone tell me -
how to clone this repository?
how to submit a patch?

I am a beginner.
Thank you
If you're interested in getting started on Firefox development here is a good place to start: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Simple_Firefox_build

Comment 6

a year ago
Can someone tell me how I can run the test to check if this bug has been fixed? I ran the './mach xpcshell-test services/common/tests/unit/' but it fails 1 test even without my changes.

(newbie here)
(Reporter)

Comment 7

a year ago
I don't think you need to run the tests, this should be transparent!

Comment 8

a year ago
I am done with the changes....can anyone tell me how to submit it as a patch?
Thank you

Comment 10

a year ago
Sir, can u please list the steps for a first patch contribution, there are lots of documentation but I am finding them a bit confusing. It will be of great help. Thank you
(Reporter)

Comment 11

a year ago
The exercise here is exactly about that: learning how to read documentations and use it.
Just use the Bugzilla method explained in the link from comment #9. It should be enough!
(Assignee)

Comment 13

a year ago
(In reply to Venkatesh Prabhu :vprabhu from comment #12)
> Created attachment 8954339 [details]
> Bug 1437893 - Removed an unnecessary string initialization in
> cdm-test-decryptor.cpp
> 
> Review commit: https://reviewboard.mozilla.org/r/223442/diff/#index_header
> See other reviews: https://reviewboard.mozilla.org/r/223442/

I have made the change. Please let me know if that is what was needed.
(Reporter)

Comment 14

a year ago
mozreview-review
Comment on attachment 8954339 [details]
Bug 1437893 - Removed an unnecessary string initialization in cdm-test-decryptor.cpp

https://reviewboard.mozilla.org/r/223442/#review229540

::: dom/media/fake-cdm/cdm-test-decryptor.cpp:429
(Diff revision 1)
>    ShutdownTimeout,
>    ShutdownStoreToken
>  };
>  
>  static ShutdownMode sShutdownMode = ShutdownNormal;
> -static string sShutdownToken = "";
> +static string sShutdownToken ;

No spaces before the ";"
Comment hidden (mozreview-request)
(Assignee)

Comment 16

a year ago
(In reply to Sylvestre Ledru [:sylvestre] from comment #14)
> Comment on attachment 8954339 [details]
> Bug 1437893 - Removed an unnecessary string initialization in
> cdm-test-decryptor.cpp
> 
> https://reviewboard.mozilla.org/r/223442/#review229540
> 
> ::: dom/media/fake-cdm/cdm-test-decryptor.cpp:429
> (Diff revision 1)
> >    ShutdownTimeout,
> >    ShutdownStoreToken
> >  };
> >  
> >  static ShutdownMode sShutdownMode = ShutdownNormal;
> > -static string sShutdownToken = "";
> > +static string sShutdownToken ;
> 
> No spaces before the ";"

Done. Sorry about that. I thought of that after pushing the changes for review.
(Reporter)

Updated

a year ago
Assignee: nobody → venkateshprabhu2
(Reporter)

Comment 17

a year ago
No worries! 
Now, ask for a review to cpearce (the code owner)
(Assignee)

Updated

a year ago
Attachment #8954339 - Flags: review?(cpearce)

Comment 18

a year ago
mozreview-review
Comment on attachment 8954339 [details]
Bug 1437893 - Removed an unnecessary string initialization in cdm-test-decryptor.cpp

https://reviewboard.mozilla.org/r/223442/#review229674

Thank you for the patch!
Attachment #8954339 - Flags: review?(cpearce) → review+
Venkatesh: If you can mark the issue about the extra space that Sylvestre pointed out as resolved in MozReview, our patch landing system can push this code to our main repository.
(Assignee)

Comment 20

a year ago
(In reply to Chris Pearce (:cpearce) from comment #19)
> Venkatesh: If you can mark the issue about the extra space that Sylvestre
> pointed out as resolved in MozReview, our patch landing system can push this
> code to our main repository.

Done.
(Reporter)

Comment 21

a year ago
And I just landed it for you. Should be in Firefox 60.
Bravo!

Comment 22

a year ago
Pushed by sledru@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7ce5b5962798
Removed an unnecessary string initialization in cdm-test-decryptor.cpp r=cpearce

Comment 23

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/7ce5b5962798
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox60: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
You need to log in before you can comment on or make changes to this bug.