Closed Bug 1669833 Opened 4 years ago Closed 4 years ago

nsChromeRegistry.cpp: do not use 'else' after 'return'

Categories

(Core :: General, task)

task

Tracking

()

RESOLVED FIXED
83 Branch
Tracking Status
firefox83 --- fixed

People

(Reporter: Sylvestre, Assigned: baka)

References

(Blocks 1 open bug)

Details

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

Attachments

(1 file, 2 obsolete files)

Filling as a good first bug to learn workflows.

do not use 'else' after 'return':
https://searchfox.org/mozilla-central/source/dom/base/ContentIterator.cpp#267-269

As the change is trivial, it is just to learn how to contribute to Firefox.

Found by http://clang.llvm.org/extra/clang-tidy/checks/readability-else-after-return.html

Tutorial to contribute:
https://firefox-source-docs.mozilla.org/contributing/contribution_quickref.html

Please don't ask for the bug to be assigned. It will be automatically assigned to the first patch.

Assignee: nobody → akshat.dixit71
Status: NEW → ASSIGNED
Attachment #9182162 - Attachment description: Bug 1669833 - Removed else after the return statement. r?jfkthame → Bug 1669833 - Removed else after the return statement. r=jfkthame
Attachment #9182162 - Attachment description: Bug 1669833 - Removed else after the return statement. r=jfkthame → Bug 1669833 - Removed else after the return statement. r?jfkthame

Depends on D93873

Pushed by apavel@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/39a48c3abc33 Removed else after the return statement. r=jfkthame
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 83 Branch

It seems the patch from akshat.dixit71 actually fixed Bug 1669834 (nsChromeRegistry.cpp) rather than this Bug 1669833 (ContentIterator.cpp). Should this one be reopened?

I just submitted a patch for ContentIterator.cpp, which is my first contribution! I also found Bug 1658273 suggesting that this specific case may be incorrectly flagged as a problem. However, there are also other cases of [readability-else-after-return] in the same file which are not as debatable, so I decided to still submit the patch.

Flags: needinfo?(sledru)

(In reply to Rafal Bielski from comment #6)

It seems the patch from akshat.dixit71 actually fixed Bug 1669834 (nsChromeRegistry.cpp) rather than this Bug 1669833 (ContentIterator.cpp). Should this one be reopened?

I think it would make the most sense to file a new bug for ContentIterator, and attach your patch to that bug. Thanks.

I'll rename this bug to mention what actually ended up being fixed here.

Summary: ContentIterator.cpp: do not use 'else' after 'return' → nsChromeRegistry.cpp: do not use 'else' after 'return'
Flags: needinfo?(sledru)

I just went ahead and filed bug 1673913. You can attach your patch there, please. Thanks.

Flags: needinfo?(rafbiels)

Comment on attachment 9182829 [details]
Bug 1669833 - Fix readability-else-after-return warnings. r?masayuki

Revision D94243 was moved to bug 1673913. Setting attachment 9182829 [details] to obsolete.

Attachment #9182829 - Attachment is obsolete: true

Thank you Andrew, I moved the patch to Bug 1673913

Flags: needinfo?(rafbiels)
Attachment #9182166 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: