1338086 - Remove useless else blocks in order to reduce complexity

Assignee

Description

•

8 years ago

Trying this checkers (with automatic rewrite): http://clang.llvm.org/extra/clang-tidy/checks/readability-else-after-return.html It can transform such code: bool nsViewManager::IsViewInserted(nsView *aView) { if (mRootView == aView) { return true; } else if (aView->GetParent() == nullptr) { return false; } else { nsView* view = aView->GetParent()->GetFirstChild(); while (view != nullptr) { if (view == aView) { return true; } view = view->GetNextSibling(); } return false; } } into bool nsViewManager::IsViewInserted(nsView *aView) { if (mRootView == aView) { return true; } if (aView->GetParent() == nullptr) { return false; } nsView* view = aView->GetParent()->GetFirstChild(); while (view != nullptr) { if (view == aView) { return true; } view = view->GetNextSibling(); } return false; }

Sylvestre Ledru [:Sylvestre]

Assignee

Comment 1

•

8 years ago

This is part of the evaluation of the reliability of checkers to integrate them more into the workflow.

Comment hidden (mozreview-request)

Cameron McCormack (:heycam)

Comment 39

•

8 years ago

mozreview-review

Comment on attachment 8835412 [details] Bug 1338086 - Remove useless else blocks in order to reduce complexity in layout/ https://reviewboard.mozilla.org/r/111134/#review112308 ::: layout/style/nsCSSRuleProcessor.cpp:2187 (Diff revision 1) > else if (attr->mFunction == NS_ATTR_FUNC_EQUALS) { > result = > aElement-> > AttrValueIs(attr->mNameSpace, matchAttribute, attr->mValue, > attr->IsValueCaseSensitive(isHTML) ? eCaseMatters > : eIgnoreCase); Nit: Please preserve the indentation here.

Attachment #8835412 - Flags: review?(cam) → review+

Timothy Nikkel (:tnikkel)

Comment 40

•

8 years ago

mozreview-review

Comment on attachment 8835416 [details] Bug 1338086 - Remove useless else blocks in order to reduce complexity in view/ https://reviewboard.mozilla.org/r/111142/#review112310

Attachment #8835416 - Flags: review?(tnikkel) → review+

Alessio Placitelli [:Dexter]

Comment 41

•

8 years ago

mozreview-review

Comment on attachment 8835419 [details] Bug 1338086 - Remove useless else blocks in order to reduce complexity in toolkit/components/telemetry/ https://reviewboard.mozilla.org/r/111148/#review112312

Attachment #8835419 - Flags: review?(alessio.placitelli) → review+

Sylvestre Ledru [:Sylvestre]

Assignee

Comment 42

•

8 years ago

(In reply to Cameron McCormack (:heycam) from comment #39) > Nit: Please preserve the indentation here. Yeah, sorry, I run clang-format on the diff. I will fix that.

Joel Maher ( :jmaher ) (UTC -8)

Comment 43

•

8 years ago

mozreview-review

Comment on attachment 8835420 [details] Bug 1338086 - Remove useless else blocks in order to reduce complexity testing/mochitest/ssltunnel/ https://reviewboard.mozilla.org/r/111150/#review112316 great stuff

Attachment #8835420 - Flags: review?(jmaher) → review+

Benjamin Bouvier [:bbouvier] (inactive)

Comment 44

•

8 years ago

mozreview-review

Comment on attachment 8835411 [details] Bug 1338086 - Remove useless else blocks in order to reduce complexity in js/ https://reviewboard.mozilla.org/r/111132/#review112318 I've stopped reviewing at some point: JS code style allows us to have code up to 99 columns, comments up to 79 columns. This patch contains a lot of unnecessary line-wrapping that I'd like to see removed before r+ing this. ::: js/xpconnect/loader/mozJSSubScriptLoader.cpp:165 (Diff revision 1) > - return false; > + return false; > - } > + } > - return JS::CompileFunction(cx, envChain, options, nullptr, 0, nullptr, > + return JS::CompileFunction(cx, envChain, options, nullptr, 0, nullptr, > - srcBuf, function); > + srcBuf, function); > - } > } else { Heh, still an else-after-return here :) Guess your tool should be re-run until you reach a fixed-point. ::: js/xpconnect/wrappers/XrayWrapper.cpp:561 (Diff revision 1) > + if (IsTypedArrayKey(key)) { > - if (IsArrayIndex(GetArrayIndexFromId(cx, id))) { > + if (IsArrayIndex(GetArrayIndexFromId(cx, id))) { > - // WebExtensions can't use cloneInto(), so we just let them do > + // WebExtensions can't use cloneInto(), so we just let them do > - // the slow thing to maximize compatibility. > + // the slow thing to maximize compatibility. > - if (CompartmentPrivate::Get(CurrentGlobalOrNull(cx))->isWebExtensionContentScript) { > + if (CompartmentPrivate::Get(CurrentGlobalOrNull(cx)) > + ->isWebExtensionContentScript) { nit: keep all of this one one line please. ::: js/xpconnect/wrappers/XrayWrapper.cpp:567 (Diff revision 1) > - Rooted<PropertyDescriptor> innerDesc(cx); > + Rooted<PropertyDescriptor> innerDesc(cx); > - { > + { > - JSAutoCompartment ac(cx, target); > + JSAutoCompartment ac(cx, target); > - JS_MarkCrossZoneId(cx, id); > + JS_MarkCrossZoneId(cx, id); > - if (!JS_GetOwnPropertyDescriptorById(cx, target, id, &innerDesc)) > + if (!JS_GetOwnPropertyDescriptorById( > + cx, target, id, &innerDesc)) ditto ::: js/xpconnect/wrappers/XrayWrapper.cpp:571 (Diff revision 1) > - if (!JS_GetOwnPropertyDescriptorById(cx, target, id, &innerDesc)) > + if (!JS_GetOwnPropertyDescriptorById( > + cx, target, id, &innerDesc)) > - return false; > + return false; > - } > + } > - if (innerDesc.isDataDescriptor() && innerDesc.value().isNumber()) { > + if (innerDesc.isDataDescriptor() && > + innerDesc.value().isNumber()) { ditto ::: js/xpconnect/wrappers/XrayWrapper.cpp:582 (Diff revision 1) > - JS_ReportErrorASCII(cx, "Accessing TypedArray data over Xrays is slow, and forbidden " > + JS_ReportErrorASCII( > + cx, > + "Accessing TypedArray data over Xrays is slow, and forbidden " > - "in order to encourage performant code. To copy TypedArrays " > + "in order to encourage performant code. To copy TypedArrays " > - "across origin boundaries, consider using Components.utils.cloneInto()."); > + "across origin boundaries, consider using " > + "Components.utils.cloneInto()."); Keep the original formatting here too, please. ::: js/xpconnect/wrappers/XrayWrapper.cpp:590 (Diff revision 1) > } else if (key == JSProto_Function) { > - if (id == GetJSIDByIndex(cx, XPCJSContext::IDX_LENGTH)) { > + if (id == GetJSIDByIndex(cx, XPCJSContext::IDX_LENGTH)) { > - FillPropertyDescriptor(desc, wrapper, JSPROP_PERMANENT | JSPROP_READONLY, > + FillPropertyDescriptor( > + desc, > + wrapper, > + JSPROP_PERMANENT | JSPROP_READONLY, ditto ::: js/xpconnect/wrappers/XrayWrapper.cpp:596 (Diff revision 1) > - NumberValue(JS_GetFunctionArity(JS_GetObjectFunction(target)))); > + NumberValue(JS_GetFunctionArity(JS_GetObjectFunction(target)))); > - return true; > + return true; > - } else if (id == GetJSIDByIndex(cx, XPCJSContext::IDX_NAME)) { > - RootedString fname(cx, JS_GetFunctionId(JS_GetObjectFunction(target))); > + } > + if (id == GetJSIDByIndex(cx, XPCJSContext::IDX_NAME)) { > + RootedString fname(cx, > + JS_GetFunctionId(JS_GetObjectFunction(target))); Stopped here.

Attachment #8835411 - Flags: review?(bbouvier)

Dragana Damjanovic [:dragana]

Comment 45

•

8 years ago

mozreview-review

Comment on attachment 8835415 [details] Bug 1338086 - Remove useless else blocks in order to reduce complexity in netwerk/ https://reviewboard.mozilla.org/r/111140/#review112320 ::: netwerk/dns/nsEffectiveTLDService.cpp:290 (Diff revision 1) > if (entry->IsWild() && prevDomain) { > // wildcard rules imply an eTLD one level inferior to the match. > eTLD = prevDomain; > break; > > - } else if (entry->IsNormal() || !nextDot) { > + } if (entry->IsNormal() || !nextDot) { move 'if (...' into a new line.

Attachment #8835415 - Flags: review?(dd.mozilla) → review+

Jean-Yves Avenard [:jya]

Comment 46

•

8 years ago

mozreview-review-reply

Comment on attachment 8835411 [details] Bug 1338086 - Remove useless else blocks in order to reduce complexity in js/ https://reviewboard.mozilla.org/r/111132/#review112318 > Heh, still an else-after-return here :) Guess your tool should be re-run until you reach a fixed-point. note that our coding style requires the use of brackets, even for single line operation. https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style#Control_Structures

Benjamin Bouvier [:bbouvier] (inactive)

Comment 47

•

8 years ago

mozreview-review-reply

Comment on attachment 8835411 [details] Bug 1338086 - Remove useless else blocks in order to reduce complexity in js/ https://reviewboard.mozilla.org/r/111132/#review112318 > note that our coding style requires the use of brackets, even for single line operation. > https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style#Control_Structures Also note Spidermonkey uses its own style: https://wiki.mozilla.org/JavaScript:SpiderMonkey:C_Coding_Style Not sure if xpconnect's style is the same as Spidermonkey. A good rule of thumb would be to reuse the same style as in this file to keep it consistent.

Andrea Marchesini [:baku]

Comment 48

•

8 years ago

mozreview-review

Comment on attachment 8835401 [details] Bug 1338086 - Remove useless else blocks in order to reduce complexity in dom/browser-element/ https://reviewboard.mozilla.org/r/111112/#review112334 ::: dom/browser-element/BrowserElementParent.cpp:173 (Diff revision 1) > val, &status); > > if (dispatchSucceeded) { > if (aPopupFrameElement->IsInUncomposedDoc()) { > return BrowserElementParent::OPEN_WINDOW_ADDED; > - } else if (status == nsEventStatus_eConsumeNoDefault) { > + } if (status == nsEventStatus_eConsumeNoDefault) { if() in a new line.

Attachment #8835401 - Flags: review?(amarchesini) → review+

Andrea Marchesini [:baku]

Comment 49

•

8 years ago

mozreview-review

Comment on attachment 8835402 [details] Bug 1338086 - Remove useless else blocks in order to reduce complexity in dom/canvas https://reviewboard.mozilla.org/r/111114/#review112336

Attachment #8835402 - Flags: review?(amarchesini) → review+

Andrea Marchesini [:baku]

Comment 50

•

8 years ago

mozreview-review

Comment on attachment 8835403 [details] Bug 1338086 - Remove useless else blocks in order to reduce complexity in dom/ipc https://reviewboard.mozilla.org/r/111116/#review112338

Attachment #8835403 - Flags: review?(amarchesini) → review+

Jean-Yves Avenard [:jya]

Comment 51

•

8 years ago

mozreview-review

Comment on attachment 8835407 [details] Bug 1338086 - Remove useless else blocks in order to reduce complexity in dom/media/ https://reviewboard.mozilla.org/r/111124/#review112340 ::: dom/media/fmp4/MP4Demuxer.cpp:401 (Diff revision 1) > aNumSamples--; > } > > if (samples->mSamples.IsEmpty()) { > - return SamplesPromise::CreateAndReject(NS_ERROR_DOM_MEDIA_END_OF_STREAM, __func__); > - } else { > + return SamplesPromise::CreateAndReject(NS_ERROR_DOM_MEDIA_END_OF_STREAM, > + __func__); fairly certain that this is no longer applicable following bug 1325707 ::: dom/media/fmp4/MP4Demuxer.cpp:413 (Diff revision 1) > - mNeedSPSForTelemetry = AccumulateSPSTelemetry(extradata); > + mNeedSPSForTelemetry = AccumulateSPSTelemetry(extradata); > - } > + } > - } > + } > > - if (mNextKeyframeTime.isNothing() || > + if (mNextKeyframeTime.isNothing() || > - samples->mSamples.LastElement()->mTime >= mNextKeyframeTime.value().ToMicroseconds()) { > + samples->mSamples.LastElement()->mTime >= operator >= is to be on the next line and aligned with the first operand.

Gian-Carlo Pascutto [:gcp]

Comment 52

•

8 years ago

mozreview-review

Comment on attachment 8835426 [details] Bug 1338086 - Remove useless else blocks in order to reduce complexity in security/sandbox/linux/ https://reviewboard.mozilla.org/r/111162/#review112348

Attachment #8835426 - Flags: review?(gpascutto) → review+

Nicolas Silva [:nical]

Comment 53

•

8 years ago

mozreview-review

Comment on attachment 8835406 [details] Bug 1338086 - Remove useless else blocks in order to reduce complexity in gfx/ https://reviewboard.mozilla.org/r/111122/#review112350 ::: gfx/2d/image_operations.cpp:196 (Diff revision 1) > SkBitmap ImageOperations::Resize(const SkBitmap& source, > ResizeMethod method, > int dest_width, int dest_height, > const SkIRect& dest_subset, > void* dest_pixels /* = nullptr */) { > if (method == ImageOperations::RESIZE_SUBPIXEL) Please use this as an occasion to brace this if block. ::: gfx/layers/LayerTreeInvalidation.cpp:796 (Diff revision 1) > *aGeometryChanged = true; > } > return result; > - } else { > - bool geometryChanged = (aGeometryChanged != nullptr) ? *aGeometryChanged : false; > + } > + bool geometryChanged = > + (aGeometryChanged != nullptr) ? *aGeometryChanged : false; nit: I'd rather keep this assignment in one line. ::: gfx/layers/Layers.cpp:673 (Diff revision 1) > } > > Matrix4x4 > Layer::GetLocalTransform() > { > if (HostLayer* shadow = AsHostLayer()) Please brace the if here as well (I realize I'm hijacking this patch but I care more about this than the extra else blocks).

Attachment #8835406 - Flags: review?(nical.bugzilla) → review+

Nicolas Silva [:nical]

Comment 54

•

8 years ago

mozreview-review

Comment on attachment 8835434 [details] Bug 1338086 - Remove useless else blocks in order to reduce complexity in widget/xremoteclient/ https://reviewboard.mozilla.org/r/111178/#review112354 ::: widget/xremoteclient/XRemoteClient.cpp:154 (Diff revision 1) > { > if (event->error_code == BadWindow) { > sGotBadWindow = true; > return 0; // ignored > } > - else { > + Please remove the trailin spaces here and two lines below. Also, it looks like the statement below has an extra indentation now.

Attachment #8835434 - Flags: review?(nical.bugzilla) → review+

François Marier [:francois]

Comment 55

•

8 years ago

mozreview-review

Comment on attachment 8835429 [details] Bug 1338086 - Remove useless else blocks in order to reduce complexity in toolkit/components/url-classifier/ https://reviewboard.mozilla.org/r/111168/#review112358

Attachment #8835429 - Flags: review?(francois) → review+

Nathan Froyd [:froydnj]

Comment 56

•

8 years ago

mozreview-review

Comment on attachment 8835422 [details] Bug 1338086 - Remove useless else blocks in order to reduce complexity in modules/libjar/ https://reviewboard.mozilla.org/r/111154/#review112378

Attachment #8835422 - Flags: review?(nfroyd) → review+

Nathan Froyd [:froydnj]

Comment 57

•

8 years ago

mozreview-review

Comment on attachment 8835423 [details] Bug 1338086 - Remove useless else blocks in order to reduce complexity in modules/libpref/ https://reviewboard.mozilla.org/r/111156/#review112380

Attachment #8835423 - Flags: review?(nfroyd) → review+

Nathan Froyd [:froydnj]

Comment 58

•

8 years ago

mozreview-review

Comment on attachment 8835424 [details] Bug 1338086 - Remove useless else blocks in order to reduce complexity in mozglue/misc/ https://reviewboard.mozilla.org/r/111158/#review112382

Attachment #8835424 - Flags: review?(nfroyd) → review+

Nathan Froyd [:froydnj]

Comment 59

•

8 years ago

mozreview-review

Comment on attachment 8835432 [details] Bug 1338086 - Remove useless else blocks in order to reduce complexity in uriloader/exthandler/unix/ https://reviewboard.mozilla.org/r/111174/#review112384 ::: uriloader/exthandler/unix/nsOSHelperAppService.cpp:1117 (Diff revision 1) > > if (match) { // we did not fail any test clauses; all is good > // get out of here > mailcapFile->Close(); > return NS_OK; > - } else { // pretend that this match never happened > + } // pretend that this match never happened Please move the comment to the next line, on its own line.

Attachment #8835432 - Flags: review?(nfroyd) → review+

Nathan Froyd [:froydnj]

Comment 60

•

8 years ago

mozreview-review

Comment on attachment 8835417 [details] Bug 1338086 - Remove useless else blocks in order to reduce complexity in xpcom/ https://reviewboard.mozilla.org/r/111144/#review112374 No objection in principle to this change, but r- for all the unrelated formatting changes (wrapping, indentation, changing the spaces and whatnot in individual lines). ::: xpcom/build/FileLocation.cpp:197 (Diff revision 1) > - int32_t read = PR_Read(mFd, aBuf + totalRead, > + int32_t read = PR_Read(mFd, > + aBuf + totalRead, Please revert this change. ::: xpcom/build/FileLocation.cpp:208 (Diff revision 1) > - nsZipCursor cursor(mItem, mZip, reinterpret_cast<uint8_t*>(aBuf), > - aLen, true); > + nsZipCursor cursor( > + mItem, mZip, reinterpret_cast<uint8_t*>(aBuf), aLen, true); Please revert this change. ::: xpcom/build/FileLocation.cpp:213 (Diff revision 1) > - nsZipArchive::sFileCorruptedReason = "FileLocation::Data: insufficient data"; > + nsZipArchive::sFileCorruptedReason = > + "FileLocation::Data: insufficient data"; Please revert this change. ::: xpcom/reflect/xptcall/md/unix/xptcstubs_x86_64_linux.cpp:74 (Diff revision 1) > - if (nr_fpr < FPR_COUNT) > + if (nr_fpr < FPR_COUNT) > - dp->val.d = fpregs[nr_fpr++]; > + dp->val.d = fpregs[nr_fpr++]; > - else > + else > - dp->val.d = *(double*) ap++; > + dp->val.d = *(double*)ap++; Some of the indentation changes in this file are appropriate...and some are not. Please review this file (or turn the clang-format pass off, or...) before the next review.

Attachment #8835417 - Flags: review?(nfroyd) → review-

Axel Hecht [:Pike]

Comment 61

•

8 years ago

mozreview-review

Comment on attachment 8835425 [details] Bug 1338086 - Remove useless else blocks in order to reduce complexity in rdf/util/ https://reviewboard.mozilla.org/r/111160/#review112420

Attachment #8835425 - Flags: review?(l10n) → review+

Sylvestre Ledru [:Sylvestre]

Assignee

Updated

•

8 years ago

Depends on: 1338171

Randell Jesup [:jesup] (needinfo me)

Comment 62

•

8 years ago

mozreview-review

Comment on attachment 8835413 [details] Bug 1338086 - Remove useless else blocks in order to reduce complexity in media/webrtc/signaling/ https://reviewboard.mozilla.org/r/111136/#review112428 r- for style changes and for changing a directory of imported code (I'll ping bwc to review though since he handles that imported code, and JSEP) ::: media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp:2503 (Diff revision 1) > + if (mCurrentLocalDescription) { > return mCurrentLocalDescription.get(); > } > > return nullptr; Since this is a UniquePtr, wouldn't this be just "return mCurrentLocalDescription.get()"? ::: media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp:2516 (Diff revision 1) > + if (mCurrentRemoteDescription) { > return mCurrentRemoteDescription.get(); > } > > return nullptr; ditto ::: media/webrtc/signaling/src/media-conduit/AudioConduit.cpp:553 (Diff revision 1) > - if(!success) > + if (!success) { > - { Is this supposed to be a style-rewrite of these files as well? While I wouldn't be against a style-fixing pass (especially for files with mixed styles), I'd suggest keeping them separate. ::: media/webrtc/signaling/src/media-conduit/AudioConduit.cpp:1003 (Diff revision 1) > - if(GetNum10msSamplesForFrequency(freq)) > + if (GetNum10msSamplesForFrequency(freq)) { > - { > - return true; > + return true; > - } else { > - return false; > - } > + } > + return false; return !!GetNum10msSamplesForFrequency(freq); Or != 0 if that's clearer than !! ::: media/webrtc/signaling/src/media-conduit/AudioConduit.cpp:1013 (Diff revision 1) > - switch(samplingFreqHz) > - { > - case 16000: return 160; //160 samples > - case 32000: return 320; //320 samples > - case 44100: return 441; //441 samples > - case 48000: return 480; //480 samples > - default: return 0; // invalid or unsupported > + switch (samplingFreqHz) { > + case 16000: > + return 160; //160 samples > + case 32000: > + return 320; //320 samples > + case 44100: > + return 441; //441 samples > + case 48000: > + return 480; //480 samples > + default: > + return 0; // invalid or unsupported > - } > + } This is all style ::: media/webrtc/signaling/src/media-conduit/VideoConduit.cpp:314 (Diff revision 1) > if ("VP8" == name) { > return webrtc::VideoEncoder::EncoderType::kVp8; > - } else if ("VP9" == name) { > + } > + if ("VP9" == name) { > return webrtc::VideoEncoder::EncoderType::kVp9; > - } else if ("H264" == name) { > + } > + if ("H264" == name) { > return webrtc::VideoEncoder::EncoderType::kH264; > } I actually find the original more obvious here, and I believe there's no performance penalty for this. ::: media/webrtc/signaling/src/media-conduit/VideoConduit.cpp:385 (Diff revision 1) > if ("VP8" == name) { > return webrtc::VideoDecoder::DecoderType::kVp8; > - } else if ("VP9" == name) { > + } > + if ("VP9" == name) { > return webrtc::VideoDecoder::DecoderType::kVp9; > - } else if ("H264" == name) { > + } > + if ("H264" == name) { > return webrtc::VideoDecoder::DecoderType::kH264; > } > > return webrtc::VideoDecoder::DecoderType::kUnsupportedCodec; ditto ::: media/webrtc/signaling/src/sdp/sipcc/sdp_access.c:72 (Diff revision 1) > { > - if (sdp_p->version == SDP_INVALID_VALUE) { > + if (sdp_p->version == SDP_INVALID_VALUE) { > - return (FALSE); > + return (FALSE); > - } else { > - return (TRUE); > - } > + } > + return (TRUE); This is imported code. That said, we basically maintain it, but I don't suggest running style checks on imported code. If we do want to change this, I'd make it "return sdp_t->version != SDP_INVALID_VALUE;"

Attachment #8835413 - Flags: review?(rjesup) → review-

Randell Jesup [:jesup] (needinfo me)

Comment 63

•

8 years ago

Comment on attachment 8835413 [details] Bug 1338086 - Remove useless else blocks in order to reduce complexity in media/webrtc/signaling/ Byron - see my review comments before looking

Attachment #8835413 - Flags: review?(docfaraday)

Byron Campen [:bwc]

Comment 64

•

8 years ago

mozreview-review

Comment on attachment 8835413 [details] Bug 1338086 - Remove useless else blocks in order to reduce complexity in media/webrtc/signaling/ https://reviewboard.mozilla.org/r/111136/#review112456 Please don't mess with the sipcc code like this. This invites merge hell into our lives. The rest of this looks basically fine to me.

Attachment #8835413 - Flags: review?(docfaraday) → review-

Molly Howell (she/her) (mostly inactive)

Comment 65

•

8 years ago

mozreview-review

Comment on attachment 8835421 [details] Bug 1338086 - Remove useless else blocks in order to reduce complexity modules/libmar/ https://reviewboard.mozilla.org/r/111152/#review112464

Attachment #8835421 - Flags: review?(mhowell) → review+

Sylvestre Ledru [:Sylvestre]

Assignee

Comment 66

•

8 years ago

(In reply to Byron Campen [:bwc] from comment #64) > Please don't mess with the sipcc code like this. Wasn't aware it was thirdparty code, added it to the list: bug 1338193

Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]

Comment 67

•

8 years ago

mozreview-review

Comment on attachment 8835399 [details] Bug 1338086 - Remove useless else blocks in order to reduce complexity devtools/shared/ https://reviewboard.mozilla.org/r/111108/#review112502

Attachment #8835399 - Flags: review?(nfitzgerald) → review+

Daniel Holbert [:dholbert] (PTO until 2/24)

Comment 68

•

8 years ago

mozreview-review

Comment on attachment 8835410 [details] Bug 1338086 - Remove useless else blocks in order to reduce complexity image/ https://reviewboard.mozilla.org/r/111130/#review112504 Thanks for doing this cleanup! One general nit, mostly for future reference -- so right now, it looks like every single one of these commit messages is: > Bug 1338086 - Remove useless else blocks in order to reduce complexity r=[somebody] ...and that makes it pretty much impossible for sheriffs, hg archeologists, & other interested parties to differentiate between these 37 commits in our pushlog. Ideally it would be nice if these commit messages including the patch's affected top-level directories, or something like that to indicate what differentiates them -- e.g. Bug 1338086 - Remove useless else blocks in order to reduce complexity, in /image/ r=dholbert ...or... Bug 1338086 - Remove useless else blocks in order to reduce complexity, in imagelib code r=dholbert If you feel up to doing that, gold star! But it's also probably not a big enough deal to worry much about at this point - mostly noting for future bugs of this sort. ::: image/imgLoader.cpp:1705 (Diff revision 1) > - return true; > + return true; > + > - } > +} Please drop this blank line (between the now-final return statement and the function's ending curly-brace)

Attachment #8835410 - Flags: review?(dholbert) → review+

Sylvestre Ledru [:Sylvestre]

Assignee

Comment 69

•

8 years ago

> If you feel up to doing that, gold star! Yeah, i am planning to do it ;)

(no longer active)

Comment 70

•

8 years ago

mozreview-review

Comment on attachment 8835400 [details] Bug 1338086 - Remove useless else blocks in order to reduce complexity in dom/base/ https://reviewboard.mozilla.org/r/111110/#review112536

Attachment #8835400 - Flags: review?(ehsan) → review+

(no longer active)

Comment 71

•

8 years ago

mozreview-review

Comment on attachment 8835427 [details] Bug 1338086 - Remove useless else blocks in order to reduce complexity in toolkit/components/remote/ https://reviewboard.mozilla.org/r/111164/#review112538

Attachment #8835427 - Flags: review?(ehsan) → review+

(no longer active)

Comment 72

•

8 years ago

mozreview-review

Comment on attachment 8835430 [details] Bug 1338086 - Remove useless else blocks in order to reduce complexity in toolkit/mozapps/update/updater/ https://reviewboard.mozilla.org/r/111170/#review112540

Attachment #8835430 - Flags: review?(ehsan) → review+

Markus Stange [:mstange]

Comment 73

•

8 years ago

mozreview-review

Comment on attachment 8835428 [details] Bug 1338086 - Remove useless else blocks in order to reduce complexity in toolkit/components/typeaheadfind https://reviewboard.mozilla.org/r/111166/#review112566

Attachment #8835428 - Flags: review?(mstange) → review+

Markus Stange [:mstange]

Comment 74

•

8 years ago

mozreview-review

Comment on attachment 8835433 [details] Bug 1338086 - Remove useless else blocks in order to reduce complexity widget/ https://reviewboard.mozilla.org/r/111176/#review112568

Attachment #8835433 - Flags: review?(mstange) → review+

Chris Pearce [:cpearce (Not reading bugmail)]

Comment 75

•

8 years ago

mozreview-review

Comment on attachment 8835404 [details] Bug 1338086 - Remove useless else blocks in order to reduce complexity in dom/media/gmp* https://reviewboard.mozilla.org/r/111118/#review112582

Attachment #8835404 - Flags: review?(cpearce) → review+

David Bolter [:davidb] (NeedInfo me for attention)

Comment 76

•

8 years ago

mozreview-review

Comment on attachment 8835398 [details] Bug 1338086 - Remove useless else blocks in order to reduce complexity accessible/atk https://reviewboard.mozilla.org/r/111106/#review112616 r+ with a couple of nits to fix: ::: accessible/atk/nsMaiInterfaceTable.cpp:359 (Diff revision 1) > { > AccessibleWrap* accWrap = GetAccessibleWrap(ATK_OBJECT(aTable)); > if (accWrap) { > return static_cast<gboolean>(accWrap->AsTable()-> > IsCellSelected(aRowIdx, aColIdx)); > - } else if (ProxyAccessible* proxy = GetProxy(ATK_OBJECT(aTable))) { > + } if (ProxyAccessible* proxy = GetProxy(ATK_OBJECT(aTable))) { Please put 'if' on new line. ::: accessible/atk/nsMaiInterfaceText.cpp:530 (Diff revision 1) > if (!text || !text->IsTextRole()) { > return FALSE; > } > > return text->AddToSelection(aStartOffset, aEndOffset); > - } else if (ProxyAccessible* proxy = GetProxy(ATK_OBJECT(aText))) { > + } if (ProxyAccessible* proxy = GetProxy(ATK_OBJECT(aText))) { Please put 'if' on new line.

Attachment #8835398 - Flags: review?(dbolter) → review+

Jean-Yves Avenard [:jya]

Comment 77

•

8 years ago

mozreview-review

Comment on attachment 8835405 [details] Bug 1338086 - Remove useless else blocks in order to reduce complexity in dom/media https://reviewboard.mozilla.org/r/111120/#review112632

Attachment #8835405 - Flags: review?(jyavenard) → review+

Jean-Yves Avenard [:jya]

Comment 78

•

8 years ago

mozreview-review

Comment on attachment 8835407 [details] Bug 1338086 - Remove useless else blocks in order to reduce complexity in dom/media/ https://reviewboard.mozilla.org/r/111124/#review112634

Attachment #8835407 - Flags: review?(jyavenard) → review+

Nicholas Nethercote [inactive]

Comment 79

•

8 years ago

mozreview-review

Comment on attachment 8835414 [details] Bug 1338086 - Remove useless else blocks in order to reduce complexity in memory/ https://reviewboard.mozilla.org/r/111138/#review112666 ::: memory/replace/logalloc/replay/Replay.cpp:523 (Diff revision 1) > > /* jemalloc_stats and free are functions with no result. */ > if (func == Buffer("jemalloc_stats")) { > replay.jemalloc_stats(args); > continue; > - } else if (func == Buffer("free")) { > + } if (func == Buffer("free")) { The 'if' needs to be on a separate line! Without that, this change makes the code less readable.

Attachment #8835414 - Flags: review?(n.nethercote) → review-

Sylvestre Ledru [:Sylvestre]

Assignee

Comment 80

•

8 years ago

mozreview-review

Comment on attachment 8835413 [details] Bug 1338086 - Remove useless else blocks in order to reduce complexity in media/webrtc/signaling/ https://reviewboard.mozilla.org/r/111136/#review112840 ::: media/webrtc/signaling/src/media-conduit/VideoConduit.cpp:314 (Diff revision 1) > if ("VP8" == name) { > return webrtc::VideoEncoder::EncoderType::kVp8; > - } else if ("VP9" == name) { > + } > + if ("VP9" == name) { > return webrtc::VideoEncoder::EncoderType::kVp9; > - } else if ("H264" == name) { > + } > + if ("H264" == name) { > return webrtc::VideoEncoder::EncoderType::kH264; > } I can fix that but are you sure? This is going to make the tool trigger this all the time at beginning (until we have the feature to ignore)

Jim Mathies [:jimm]

Comment 81

•

8 years ago

mozreview-review

Comment on attachment 8835408 [details] Bug 1338086 - Remove useless else blocks in order to reduce complexity in dom/plugins/ https://reviewboard.mozilla.org/r/111126/#review112870 ::: dom/plugins/base/nsPluginHost.cpp:2847 (Diff revision 1) > // There is no profile yet, this will tell us if there is going to be one > // in the future. > directoryService->Get(NS_APP_PROFILE_DIR_STARTUP, NS_GET_IID(nsIFile), > getter_AddRefs(mPluginRegFile)); > if (!mPluginRegFile) > return NS_ERROR_FAILURE; nit, add braces here

Attachment #8835408 - Flags: review?(jmathies) → review+

Jim Mathies [:jimm]

Comment 82

•

8 years ago

mozreview-review

Comment on attachment 8835431 [details] Bug 1338086 - Remove useless else blocks in order to reduce complexity in toolkit/xre/ https://reviewboard.mozilla.org/r/111172/#review112872

Attachment #8835431 - Flags: review?(jmathies) → review+

Daniel Holbert [:dholbert] (PTO until 2/24)

Comment 83

•

8 years ago

(Before landing, it would probably be worth grepping the whole patch-set (via "hg out -p | grep" or similar) for any lines that include "} if", to hopefully catch any instances that reviewers might not have already pointed out. It seems like that's an antipattern that the semi-automated process here produced repeatedly, so it'd be nice to be sure you've caught all of them.)

Sylvestre Ledru [:Sylvestre]

Assignee

Comment 84

•

8 years ago

(In reply to Daniel Holbert [:dholbert] from comment #83) > (Before landing, it would probably be worth grepping the whole patch-set > (via "hg out -p | grep" or similar) for any lines that include "} if", to > hopefully catch any instances that reviewers might not have already pointed > out. I fixed everything yesterday but I cannot push to mozreview because of bug 1338530 :( sorry about that I also reported a bug upstream about that https://llvm.org/bugs/show_bug.cgi?id=31915 even if upstream is already aware of that: https://github.com/llvm-mirror/clang-tools-extra/blob/master/clang-tidy/readability/ElseAfterReturnCheck.cpp#L48

Nils Ohlmeier [:drno]

Comment 85

•

8 years ago

mozreview-review

Comment on attachment 8835418 [details] Bug 1338086 - Remove useless else blocks in order to reduce complexity in media/mtransport/ https://reviewboard.mozilla.org/r/111146/#review112946 ::: media/mtransport/test_nr_socket.cpp:561 (Diff revision 1) > - port_mappings_.front()->external_socket_->my_addr().as_string, > + port_mappings_.front()->external_socket_->my_addr().as_string, > - port_mappings_.front()->remote_address_.as_string); > + port_mappings_.front()->remote_address_.as_string); > - > - port_mappings_.front()->last_used_ = PR_IntervalNow(); > + port_mappings_.front()->last_used_ = PR_IntervalNow(); > - return port_mappings_.front()->external_socket_->write(msg, len, written); > + return port_mappings_.front()->external_socket_->write(msg, len, written); > + Please remove this added empty line. ::: media/mtransport/test_nr_socket.cpp:908 (Diff revision 1) > > if (r == R_WOULDBLOCK) { > r_log(LOG_GENERIC, LOG_DEBUG, "Enqueueing UDP packet to %s", to.as_string); > send_queue_.push_back(RefPtr<UdpPacket>(new UdpPacket(msg, len, to))); > return 0; > - } else if (r) { > + } if (r) { This should go onto a new line.

Attachment #8835418 - Flags: review?(drno) → review+

Randell Jesup [:jesup] (needinfo me)

Comment 86

•

8 years ago

mozreview-review-reply

Comment on attachment 8835413 [details] Bug 1338086 - Remove useless else blocks in order to reduce complexity in media/webrtc/signaling/ https://reviewboard.mozilla.org/r/111136/#review112840 > I can fix that but are you sure? This is going to make the tool trigger this all the time at beginning (until we have the feature to ignore) That's the downside of using automated tools like this: you can ignore them, but then they'll bug you to the end of time. So either we need a "I know what I'm doing" block for the tool, or you give in. While I find the 'else if ("foo" == bar) {' form clearer, It's not worth manually ignoring it all the time. If we had a suppression mechanism (or rather if the tool did), that'd work. Go ahead and change it

(no longer active)

Comment 87

•

8 years ago

(In reply to Randell Jesup [:jesup] from comment #86) > Comment on attachment 8835413 [details] > Bug 1338086 - Remove useless else blocks in order to reduce complexity > > https://reviewboard.mozilla.org/r/111136/#review112840 > > > I can fix that but are you sure? This is going to make the tool trigger this all the time at beginning (until we have the feature to ignore) > > That's the downside of using automated tools like this: you can ignore them, > but then they'll bug you to the end of time. So either we need a "I know > what I'm doing" block for the tool, or you give in. Can't you add a // NOLINT comment on the line you want clang-tidy to ignore?

Justin Dolske [:Dolske]

Comment 88

•

8 years ago

mozreview-review

Comment on attachment 8835409 [details] Bug 1338086 - Remove useless else blocks in order to reduce complexity in extensions/gio/ https://reviewboard.mozilla.org/r/111128/#review113532

Attachment #8835409 - Flags: review?(dolske) → review+

Comment hidden (mozreview-request)

Attachment #8835409 - Attachment description: Bug 1338086 - Remove useless else blocks in order to reduce complexity → Bug 1338086 - Remove useless else blocks in order to reduce complexity in extensions/gio/

Attachment #8835408 - Attachment description: Bug 1338086 - Remove useless else blocks in order to reduce complexity → Bug 1338086 - Remove useless else blocks in order to reduce complexity in dom/plugins/

Attachment #8835406 - Attachment description: Bug 1338086 - Remove useless else blocks in order to reduce complexity → Bug 1338086 - Remove useless else blocks in order to reduce complexity in gfx/

Attachment #8835405 - Attachment description: Bug 1338086 - Remove useless else blocks in order to reduce complexity → Bug 1338086 - Remove useless else blocks in order to reduce complexity in dom/media

Attachment #8835404 - Attachment description: Bug 1338086 - Remove useless else blocks in order to reduce complexity → Bug 1338086 - Remove useless else blocks in order to reduce complexity in dom/media/gmp*

Attachment #8835403 - Attachment description: Bug 1338086 - Remove useless else blocks in order to reduce complexity → Bug 1338086 - Remove useless else blocks in order to reduce complexity in dom/ipc

Attachment #8835402 - Attachment description: Bug 1338086 - Remove useless else blocks in order to reduce complexity → Bug 1338086 - Remove useless else blocks in order to reduce complexity in dom/canvas

Attachment #8835401 - Attachment description: Bug 1338086 - Remove useless else blocks in order to reduce complexity → Bug 1338086 - Remove useless else blocks in order to reduce complexity in dom/browser-element/

Attachment #8835399 - Attachment description: Bug 1338086 - Remove useless else blocks in order to reduce complexity → Bug 1338086 - Remove useless else blocks in order to reduce complexity devtools/shared/

Attachment #8835398 - Attachment description: Bug 1338086 - Remove useless else blocks in order to reduce complexity → Bug 1338086 - Remove useless else blocks in order to reduce complexity accessible/atk

Attachment #8835410 - Attachment description: Bug 1338086 - Remove useless else blocks in order to reduce complexity → Bug 1338086 - Remove useless else blocks in order to reduce complexity image/

Attachment #8835433 - Attachment description: Bug 1338086 - Remove useless else blocks in order to reduce complexity → Bug 1338086 - Remove useless else blocks in order to reduce complexity widget/

Attachment #8835432 - Attachment description: Bug 1338086 - Remove useless else blocks in order to reduce complexity → Bug 1338086 - Remove useless else blocks in order to reduce complexity in uriloader/exthandler/unix/

Attachment #8835431 - Attachment description: Bug 1338086 - Remove useless else blocks in order to reduce complexity → Bug 1338086 - Remove useless else blocks in order to reduce complexity in toolkit/xre/

Attachment #8835430 - Attachment description: Bug 1338086 - Remove useless else blocks in order to reduce complexity → Bug 1338086 - Remove useless else blocks in order to reduce complexity in toolkit/mozapps/update/updater/

Attachment #8835429 - Attachment description: Bug 1338086 - Remove useless else blocks in order to reduce complexity → Bug 1338086 - Remove useless else blocks in order to reduce complexity in toolkit/components/url-classifier/

Attachment #8835428 - Attachment description: Bug 1338086 - Remove useless else blocks in order to reduce complexity → Bug 1338086 - Remove useless else blocks in order to reduce complexity in toolkit/components/typeaheadfind

Attachment #8835427 - Attachment description: Bug 1338086 - Remove useless else blocks in order to reduce complexity → Bug 1338086 - Remove useless else blocks in order to reduce complexity in toolkit/components/remote/

Attachment #8835426 - Attachment description: Bug 1338086 - Remove useless else blocks in order to reduce complexity → Bug 1338086 - Remove useless else blocks in order to reduce complexity in security/sandbox/linux/

Attachment #8835425 - Attachment description: Bug 1338086 - Remove useless else blocks in order to reduce complexity → Bug 1338086 - Remove useless else blocks in order to reduce complexity in rdf/util/

Attachment #8835424 - Attachment description: Bug 1338086 - Remove useless else blocks in order to reduce complexity → Bug 1338086 - Remove useless else blocks in order to reduce complexity in mozglue/misc/

Attachment #8835423 - Attachment description: Bug 1338086 - Remove useless else blocks in order to reduce complexity → Bug 1338086 - Remove useless else blocks in order to reduce complexity in modules/libpref/

Attachment #8835422 - Attachment description: Bug 1338086 - Remove useless else blocks in order to reduce complexity → Bug 1338086 - Remove useless else blocks in order to reduce complexity in modules/libjar/

Attachment #8835421 - Attachment description: Bug 1338086 - Remove useless else blocks in order to reduce complexity → Bug 1338086 - Remove useless else blocks in order to reduce complexity modules/libmar/

Attachment #8835420 - Attachment description: Bug 1338086 - Remove useless else blocks in order to reduce complexity → Bug 1338086 - Remove useless else blocks in order to reduce complexity testing/mochitest/ssltunnel/

Attachment #8835419 - Attachment description: Bug 1338086 - Remove useless else blocks in order to reduce complexity → Bug 1338086 - Remove useless else blocks in order to reduce complexity in toolkit/components/telemetry/

Attachment #8835418 - Attachment description: Bug 1338086 - Remove useless else blocks in order to reduce complexity → Bug 1338086 - Remove useless else blocks in order to reduce complexity in media/mtransport/

Attachment #8835417 - Attachment description: Bug 1338086 - Remove useless else blocks in order to reduce complexity → Bug 1338086 - Remove useless else blocks in order to reduce complexity in xpcom/

Attachment #8835416 - Attachment description: Bug 1338086 - Remove useless else blocks in order to reduce complexity → Bug 1338086 - Remove useless else blocks in order to reduce complexity in view/

Attachment #8835415 - Attachment description: Bug 1338086 - Remove useless else blocks in order to reduce complexity → Bug 1338086 - Remove useless else blocks in order to reduce complexity in netwerk/

Attachment #8835414 - Attachment description: Bug 1338086 - Remove useless else blocks in order to reduce complexity → Bug 1338086 - Remove useless else blocks in order to reduce complexity in memory/

Attachment #8835412 - Attachment description: Bug 1338086 - Remove useless else blocks in order to reduce complexity → Bug 1338086 - Remove useless else blocks in order to reduce complexity in layout/

Attachment #8835411 - Attachment description: Bug 1338086 - Remove useless else blocks in order to reduce complexity → Bug 1338086 - Remove useless else blocks in order to reduce complexity in js/

Attachment #8835434 - Attachment description: Bug 1338086 - Remove useless else blocks in order to reduce complexity → Bug 1338086 - Remove useless else blocks in order to reduce complexity in widget/xremoteclient/

Attachment #8835407 - Attachment description: Bug 1338086 - Remove useless else blocks in order to reduce complexity → Bug 1338086 - Remove useless else blocks in order to reduce complexity in dom/media/

Attachment #8835400 - Attachment description: Bug 1338086 - Remove useless else blocks in order to reduce complexity → Bug 1338086 - Remove useless else blocks in order to reduce complexity in dom/base/

Attachment #8835413 - Attachment description: Bug 1338086 - Remove useless else blocks in order to reduce complexity → Bug 1338086 - Remove useless else blocks in order to reduce complexity in media/webrtc/signaling/

Attachment #8835417 - Flags: review- → review?(nfroyd)

Attachment #8835414 - Flags: review- → review?(n.nethercote)

Attachment #8835411 - Flags: review?(bbouvier)

Attachment #8835413 - Flags: review?(rjesup)

Attachment #8835413 - Flags: review-

Comment hidden (mozreview-request)

Nathan Froyd [:froydnj]

Comment 126

•

8 years ago

mozreview-review

Comment on attachment 8835417 [details] Bug 1338086 - Remove useless else blocks in order to reduce complexity in xpcom/ https://reviewboard.mozilla.org/r/111144/#review113704 Thanks.

Attachment #8835417 - Flags: review?(nfroyd) → review+

Comment hidden (mozreview-request)

Benjamin Bouvier [:bbouvier] (inactive)

Comment 155

•

8 years ago

mozreview-review

Comment on attachment 8835411 [details] Bug 1338086 - Remove useless else blocks in order to reduce complexity in js/ https://reviewboard.mozilla.org/r/111132/#review113708 Indentation consists of 4 spaces in Spidermonkey, not 2. There are many unwanted space changes in this file, so can't r+ yet, sorry. ::: js/xpconnect/loader/mozJSSubScriptLoader.cpp (Diff revision 2) > - return false; > + return false; > - } > + } > - return JS::CompileFunction(cx, envChain, options, nullptr, 0, nullptr, > + return JS::CompileFunction(cx, envChain, options, nullptr, 0, nullptr, > - srcBuf, function); > + srcBuf, function); > - } > + } > - } else { nit: please add a \n before the multi-line comment ::: js/xpconnect/wrappers/XrayWrapper.cpp:582 (Diff revision 2) > - } > + } > - } > } else if (key == JSProto_Function) { > - if (id == GetJSIDByIndex(cx, XPCJSContext::IDX_LENGTH)) { > + if (id == GetJSIDByIndex(cx, XPCJSContext::IDX_LENGTH)) { > - FillPropertyDescriptor(desc, wrapper, JSPROP_PERMANENT | JSPROP_READONLY, > + FillPropertyDescriptor(desc, wrapper, JSPROP_PERMANENT | JSPROP_READONLY, > - NumberValue(JS_GetFunctionArity(JS_GetObjectFunction(target)))); > + NumberValue(JS_GetFunctionArity(JS_GetObjectFunction(target)))); nit: align `NumberValue` with `desc` above.

Attachment #8835411 - Flags: review?(bbouvier)

Comment hidden (mozreview-request)

Benjamin Bouvier [:bbouvier] (inactive)

Comment 161

•

8 years ago

mozreview-review

Comment on attachment 8835411 [details] Bug 1338086 - Remove useless else blocks in order to reduce complexity in js/ https://reviewboard.mozilla.org/r/111132/#review113730 Thanks!

Attachment #8835411 - Flags: review?(bbouvier) → review+

Comment hidden (mozreview-request)

Nicholas Nethercote [inactive]

Comment 163

•

8 years ago

mozreview-review

Comment on attachment 8835414 [details] Bug 1338086 - Remove useless else blocks in order to reduce complexity in memory/ https://reviewboard.mozilla.org/r/111138/#review113846

Attachment #8835414 - Flags: review?(n.nethercote) → review+

Randell Jesup [:jesup] (needinfo me)

Comment 164

•

8 years ago

mozreview-review

Comment on attachment 8835413 [details] Bug 1338086 - Remove useless else blocks in order to reduce complexity in media/webrtc/signaling/ https://reviewboard.mozilla.org/r/111136/#review113868 ::: media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp:1814 (Diff revisions 1 - 5) > - for (auto & mRemoteTrack : mRemoteTracks) { > - mRemoteTrack.mAssignedMLine.reset(); > + for (auto i = mRemoteTracks.begin(); i != mRemoteTracks.end(); ++i) { > + i->mAssignedMLine.reset(); > } this patch is undoing a bunch of fixes from the for() loops patch...

Attachment #8835413 - Flags: review?(rjesup) → review-

Comment hidden (mozreview-request)

Randell Jesup [:jesup] (needinfo me)

Comment 203

•

8 years ago

mozreview-review

Comment on attachment 8835413 [details] Bug 1338086 - Remove useless else blocks in order to reduce complexity in media/webrtc/signaling/ https://reviewboard.mozilla.org/r/111136/#review114156 Make the one mod I indicate, and this can land! ::: media/webrtc/signaling/src/media-conduit/AudioConduit.cpp:542 (Diff revision 7) > - //copy this to local database > + //copy this to local database > - if(CopyCodecToDB(codec)) > + if(CopyCodecToDB(codec)) > - { > + { > - success = true; > + success = true; > - } else { > + } else { > - CSFLogError(logTag,"%s Unable to updated Codec Database", __FUNCTION__); > + CSFLogError(logTag,"%s Unable to updated Codec Database", __FUNCTION__); > - return kMediaConduitUnknownError; > + return kMediaConduitUnknownError; > - } > + } > + success = true; if(!CopyCodecToDB(codec)) { CFSLogError(...); return kMediaConduitUnknownError; } success = true; ::: media/webrtc/signaling/src/media-conduit/AudioConduit.cpp:930 (Diff revision 7) > // XXX(pkerr) - the PacketOptions are being ignored. This parameter was added along > // with the Call API update in the webrtc.org codebase. > // The only field in it is the packet_id, which is used when the header > // extension for TransportSequenceNumber is being used, which we don't. > - (void) options; > - if(mTransmitterTransport && > + (void)options; > + if (mTransmitterTransport && this file is an unfortunate mix of "if (" and "if(", so I have no problem moving it towards standard moz style ("if ("). Normally I'm loathe to change stuff for no solid reason, even if mixed-usage cases like this, because it messes over Blame.

Attachment #8835413 - Flags: review?(rjesup) → review+

Comment hidden (mozreview-request)

Pulsebot

Comment 205

•

8 years ago

Pushed by sledru@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/145447470634 Remove useless else blocks in order to reduce complexity in extensions/gio/ r=Dolske https://hg.mozilla.org/integration/autoland/rev/f2690dfca9be Remove useless else blocks in order to reduce complexity in dom/plugins/ r=jimm https://hg.mozilla.org/integration/autoland/rev/9fef6905c065 Remove useless else blocks in order to reduce complexity in gfx/ r=nical https://hg.mozilla.org/integration/autoland/rev/e6ce7125efcb Remove useless else blocks in order to reduce complexity in dom/media r=jya https://hg.mozilla.org/integration/autoland/rev/ce59e8f3aaf1 Remove useless else blocks in order to reduce complexity in dom/media/gmp* r=cpearce https://hg.mozilla.org/integration/autoland/rev/81e69418454e Remove useless else blocks in order to reduce complexity in dom/ipc r=baku https://hg.mozilla.org/integration/autoland/rev/f7d85c7f485f Remove useless else blocks in order to reduce complexity in dom/canvas r=baku https://hg.mozilla.org/integration/autoland/rev/d687959b0d9a Remove useless else blocks in order to reduce complexity in dom/browser-element/ r=baku https://hg.mozilla.org/integration/autoland/rev/1d8304f5e83a Remove useless else blocks in order to reduce complexity devtools/shared/ r=fitzgen https://hg.mozilla.org/integration/autoland/rev/fc5dd8085a28 Remove useless else blocks in order to reduce complexity accessible/atk r=davidb https://hg.mozilla.org/integration/autoland/rev/e5080c5fd186 Remove useless else blocks in order to reduce complexity image/ r=dholbert https://hg.mozilla.org/integration/autoland/rev/22a9503bcd34 Remove useless else blocks in order to reduce complexity widget/ r=mstange https://hg.mozilla.org/integration/autoland/rev/fbaa986c8e67 Remove useless else blocks in order to reduce complexity in uriloader/exthandler/unix/ r=froydnj https://hg.mozilla.org/integration/autoland/rev/d4a5c7295bbd Remove useless else blocks in order to reduce complexity in toolkit/xre/ r=jimm https://hg.mozilla.org/integration/autoland/rev/d897e71f8818 Remove useless else blocks in order to reduce complexity in toolkit/mozapps/update/updater/ r=Ehsan https://hg.mozilla.org/integration/autoland/rev/8d7006b65a2c Remove useless else blocks in order to reduce complexity in toolkit/components/url-classifier/ r=francois https://hg.mozilla.org/integration/autoland/rev/e7fcd290e89e Remove useless else blocks in order to reduce complexity in toolkit/components/typeaheadfind r=mstange https://hg.mozilla.org/integration/autoland/rev/cd8319b71afd Remove useless else blocks in order to reduce complexity in toolkit/components/remote/ r=Ehsan https://hg.mozilla.org/integration/autoland/rev/3096b29c3a5b Remove useless else blocks in order to reduce complexity in security/sandbox/linux/ r=gcp https://hg.mozilla.org/integration/autoland/rev/aea7a26ac8fc Remove useless else blocks in order to reduce complexity in rdf/util/ r=Pike https://hg.mozilla.org/integration/autoland/rev/a0a07671fdb1 Remove useless else blocks in order to reduce complexity in mozglue/misc/ r=froydnj https://hg.mozilla.org/integration/autoland/rev/dbc0c3eb16b4 Remove useless else blocks in order to reduce complexity in modules/libpref/ r=froydnj https://hg.mozilla.org/integration/autoland/rev/5151fc89c210 Remove useless else blocks in order to reduce complexity in modules/libjar/ r=froydnj https://hg.mozilla.org/integration/autoland/rev/8a6881a79dad Remove useless else blocks in order to reduce complexity modules/libmar/ r=mhowell https://hg.mozilla.org/integration/autoland/rev/1c91ef325bc9 Remove useless else blocks in order to reduce complexity testing/mochitest/ssltunnel/ r=jmaher https://hg.mozilla.org/integration/autoland/rev/be741314456b Remove useless else blocks in order to reduce complexity in toolkit/components/telemetry/ r=Dexter https://hg.mozilla.org/integration/autoland/rev/a6f7b959e857 Remove useless else blocks in order to reduce complexity in media/mtransport/ r=drno https://hg.mozilla.org/integration/autoland/rev/a1411b3c461e Remove useless else blocks in order to reduce complexity in xpcom/ r=froydnj https://hg.mozilla.org/integration/autoland/rev/0775748c6e49 Remove useless else blocks in order to reduce complexity in view/ r=tnikkel https://hg.mozilla.org/integration/autoland/rev/5bf8f4e866ef Remove useless else blocks in order to reduce complexity in netwerk/ r=dragana https://hg.mozilla.org/integration/autoland/rev/2e9d9af6bd9a Remove useless else blocks in order to reduce complexity in memory/ r=njn https://hg.mozilla.org/integration/autoland/rev/9e7ec782abbc Remove useless else blocks in order to reduce complexity in layout/ r=heycam https://hg.mozilla.org/integration/autoland/rev/36a9f59888a8 Remove useless else blocks in order to reduce complexity in js/ r=bbouvier https://hg.mozilla.org/integration/autoland/rev/498ce28ac33e Remove useless else blocks in order to reduce complexity in widget/xremoteclient/ r=nical https://hg.mozilla.org/integration/autoland/rev/c37a59aa9e9d Remove useless else blocks in order to reduce complexity in dom/media/ r=jya https://hg.mozilla.org/integration/autoland/rev/11cfe50731f8 Remove useless else blocks in order to reduce complexity in dom/base/ r=Ehsan https://hg.mozilla.org/integration/autoland/rev/713d7f119dc0 Remove useless else blocks in order to reduce complexity in media/webrtc/signaling/ r=jesup

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

Comment 206

•

8 years ago

bugherder

Status: NEW → RESOLVED

Closed: 8 years ago

status-firefox54: affected → fixed

Resolution: --- → FIXED

Target Milestone: --- → mozilla54

BMO Automation

Updated

•

7 years ago

Product: Core → Firefox Build System

BMO Automation

Updated

•

3 years ago

Product: Firefox Build System → Developer Infrastructure

Bug 1338086 - Remove useless else blocks in order to reduce complexity accessible/atk 8 years ago Sylvestre Ledru [:Sylvestre] 59 bytes, text/x-review-board-request	davidb : review+	Details
Bug 1338086 - Remove useless else blocks in order to reduce complexity devtools/shared/ 8 years ago Sylvestre Ledru [:Sylvestre] 59 bytes, text/x-review-board-request	fitzgen : review+	Details
Bug 1338086 - Remove useless else blocks in order to reduce complexity in dom/base/ 8 years ago Sylvestre Ledru [:Sylvestre] 59 bytes, text/x-review-board-request	ehsan.akhgari : review+	Details
Bug 1338086 - Remove useless else blocks in order to reduce complexity in dom/browser-element/ 8 years ago Sylvestre Ledru [:Sylvestre] 59 bytes, text/x-review-board-request	baku : review+	Details
Bug 1338086 - Remove useless else blocks in order to reduce complexity in dom/canvas 8 years ago Sylvestre Ledru [:Sylvestre] 59 bytes, text/x-review-board-request	baku : review+	Details
Bug 1338086 - Remove useless else blocks in order to reduce complexity in dom/ipc 8 years ago Sylvestre Ledru [:Sylvestre] 59 bytes, text/x-review-board-request	baku : review+	Details
Bug 1338086 - Remove useless else blocks in order to reduce complexity in dom/media/gmp* 8 years ago Sylvestre Ledru [:Sylvestre] 59 bytes, text/x-review-board-request	cpearce : review+	Details
Bug 1338086 - Remove useless else blocks in order to reduce complexity in dom/media 8 years ago Sylvestre Ledru [:Sylvestre] 59 bytes, text/x-review-board-request	jya : review+	Details
Bug 1338086 - Remove useless else blocks in order to reduce complexity in gfx/ 8 years ago Sylvestre Ledru [:Sylvestre] 59 bytes, text/x-review-board-request	nical : review+	Details
Bug 1338086 - Remove useless else blocks in order to reduce complexity in dom/media/ 8 years ago Sylvestre Ledru [:Sylvestre] 59 bytes, text/x-review-board-request	jya : review+	Details
Bug 1338086 - Remove useless else blocks in order to reduce complexity in dom/plugins/ 8 years ago Sylvestre Ledru [:Sylvestre] 59 bytes, text/x-review-board-request	jimm : review+	Details
Bug 1338086 - Remove useless else blocks in order to reduce complexity in extensions/gio/ 8 years ago Sylvestre Ledru [:Sylvestre] 59 bytes, text/x-review-board-request	Dolske : review+	Details
Bug 1338086 - Remove useless else blocks in order to reduce complexity image/ 8 years ago Sylvestre Ledru [:Sylvestre] 59 bytes, text/x-review-board-request	dholbert : review+	Details
Bug 1338086 - Remove useless else blocks in order to reduce complexity in js/ 8 years ago Sylvestre Ledru [:Sylvestre] 59 bytes, text/x-review-board-request	bbouvier : review+	Details
Bug 1338086 - Remove useless else blocks in order to reduce complexity in layout/ 8 years ago Sylvestre Ledru [:Sylvestre] 59 bytes, text/x-review-board-request	heycam : review+	Details
Bug 1338086 - Remove useless else blocks in order to reduce complexity in media/webrtc/signaling/ 8 years ago Sylvestre Ledru [:Sylvestre] 59 bytes, text/x-review-board-request	jesup : review+	Details
Bug 1338086 - Remove useless else blocks in order to reduce complexity in memory/ 8 years ago Sylvestre Ledru [:Sylvestre] 59 bytes, text/x-review-board-request	n.nethercote : review+	Details
Bug 1338086 - Remove useless else blocks in order to reduce complexity in netwerk/ 8 years ago Sylvestre Ledru [:Sylvestre] 59 bytes, text/x-review-board-request	dragana : review+	Details
Bug 1338086 - Remove useless else blocks in order to reduce complexity in view/ 8 years ago Sylvestre Ledru [:Sylvestre] 59 bytes, text/x-review-board-request	tnikkel : review+	Details
Bug 1338086 - Remove useless else blocks in order to reduce complexity in xpcom/ 8 years ago Sylvestre Ledru [:Sylvestre] 59 bytes, text/x-review-board-request	froydnj : review+	Details
Bug 1338086 - Remove useless else blocks in order to reduce complexity in media/mtransport/ 8 years ago Sylvestre Ledru [:Sylvestre] 59 bytes, text/x-review-board-request	drno : review+	Details
Bug 1338086 - Remove useless else blocks in order to reduce complexity in toolkit/components/telemetry/ 8 years ago Sylvestre Ledru [:Sylvestre] 59 bytes, text/x-review-board-request	Dexter : review+	Details
Bug 1338086 - Remove useless else blocks in order to reduce complexity testing/mochitest/ssltunnel/ 8 years ago Sylvestre Ledru [:Sylvestre] 59 bytes, text/x-review-board-request	jmaher : review+	Details
Bug 1338086 - Remove useless else blocks in order to reduce complexity modules/libmar/ 8 years ago Sylvestre Ledru [:Sylvestre] 59 bytes, text/x-review-board-request	molly : review+	Details
Bug 1338086 - Remove useless else blocks in order to reduce complexity in modules/libjar/ 8 years ago Sylvestre Ledru [:Sylvestre] 59 bytes, text/x-review-board-request	froydnj : review+	Details
Bug 1338086 - Remove useless else blocks in order to reduce complexity in modules/libpref/ 8 years ago Sylvestre Ledru [:Sylvestre] 59 bytes, text/x-review-board-request	froydnj : review+	Details
Bug 1338086 - Remove useless else blocks in order to reduce complexity in mozglue/misc/ 8 years ago Sylvestre Ledru [:Sylvestre] 59 bytes, text/x-review-board-request	froydnj : review+	Details
Bug 1338086 - Remove useless else blocks in order to reduce complexity in rdf/util/ 8 years ago Sylvestre Ledru [:Sylvestre] 59 bytes, text/x-review-board-request	Pike : review+	Details
Bug 1338086 - Remove useless else blocks in order to reduce complexity in security/sandbox/linux/ 8 years ago Sylvestre Ledru [:Sylvestre] 59 bytes, text/x-review-board-request	gcp : review+	Details
Bug 1338086 - Remove useless else blocks in order to reduce complexity in toolkit/components/remote/ 8 years ago Sylvestre Ledru [:Sylvestre] 59 bytes, text/x-review-board-request	ehsan.akhgari : review+	Details
Bug 1338086 - Remove useless else blocks in order to reduce complexity in toolkit/components/typeaheadfind 8 years ago Sylvestre Ledru [:Sylvestre] 59 bytes, text/x-review-board-request	mstange : review+	Details
Bug 1338086 - Remove useless else blocks in order to reduce complexity in toolkit/components/url-classifier/ 8 years ago Sylvestre Ledru [:Sylvestre] 59 bytes, text/x-review-board-request	francois : review+	Details
Bug 1338086 - Remove useless else blocks in order to reduce complexity in toolkit/mozapps/update/updater/ 8 years ago Sylvestre Ledru [:Sylvestre] 59 bytes, text/x-review-board-request	ehsan.akhgari : review+	Details
Bug 1338086 - Remove useless else blocks in order to reduce complexity in toolkit/xre/ 8 years ago Sylvestre Ledru [:Sylvestre] 59 bytes, text/x-review-board-request	jimm : review+	Details
Bug 1338086 - Remove useless else blocks in order to reduce complexity in uriloader/exthandler/unix/ 8 years ago Sylvestre Ledru [:Sylvestre] 59 bytes, text/x-review-board-request	froydnj : review+	Details
Bug 1338086 - Remove useless else blocks in order to reduce complexity widget/ 8 years ago Sylvestre Ledru [:Sylvestre] 59 bytes, text/x-review-board-request	mstange : review+	Details
Bug 1338086 - Remove useless else blocks in order to reduce complexity in widget/xremoteclient/ 8 years ago Sylvestre Ledru [:Sylvestre] 59 bytes, text/x-review-board-request	nical : review+	Details