1149854 - Use range-based "for" loops in nsFlexContainerFrame.cpp

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Daniel Holbert [:dholbert]]

nsFlexContainerFrame has several usages of nsFrameList::Enumerator that can be cleaned up, now that nsFrameList supports range-based syntax (added in bug 1143513).

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: fix v1

This builds, which I think means it's correct.

Comment 2

[Xidorn Quan [:xidorn] UTC+10]

Comment on attachment 8586506 [details] [diff] [review]
fix v1

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

Looks good. r=me if I have permission to review patches :)

Comment 3

[Daniel Holbert [:dholbert]]

Our "permission to review patches" is a bit fuzzy and depends on the code in question.

Two general heuristics, for future "can I review this" wonderings:

* If a module owner or peer asks you to review something (as is the case here), they're asserting that you're qualified to review it, so it should be fine (though it's within your rights to disagree & punt the review to someone else :)).

* If you've worked a lot on a feature, you're probably also implicitly fine to review changes there. (This patch here sort-of falls into that category, since the patch is *making use* of a feature that you added support for. So, you're likely the best-qualified person to sign off on it. Though -- if you received a similar review request in /netwerk, you'd probably want to be sure a module peer/owner of that code was OK with you reviewing those changes in that module before granting r+.)

Comment 4

[Daniel Holbert [:dholbert]]

Also: thanks for the review! :)

Comment 5

[Daniel Holbert [:dholbert]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d9687f023f9f

Comment 6

[Xidorn Quan [:xidorn] UTC+10]

(In reply to Daniel Holbert [:dholbert] from comment #3)
> Our "permission to review patches" is a bit fuzzy and depends on the code in
> question.
> 
> Two general heuristics, for future "can I review this" wonderings:
> 
> * If a module owner or peer asks you to review something (as is the case
> here), they're asserting that you're qualified to review it, so it should be
> fine (though it's within your rights to disagree & punt the review to
> someone else :)).
> 
> * If you've worked a lot on a feature, you're probably also implicitly fine
> to review changes there. (This patch here sort-of falls into that category,
> since the patch is *making use* of a feature that you added support for. So,
> you're likely the best-qualified person to sign off on it. Though -- if you
> received a similar review request in /netwerk, you'd probably want to be
> sure a module peer/owner of that code was OK with you reviewing those
> changes in that module before granting r+.)

Thanks for explaining that!

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/d9687f023f9f