I think this is just code-style. I much prefer to drop out of control structures before returning, but also the variant parts of the statement have already been abstracted out with a variable, so I am at a loss to explain why the two identical statements haven’t been combined.

CommentFileSizeAuthor
#7 toolbar-cs.patch1 KBzeta ζ
#2 toolbar-cs.patch1 KBzeta ζ
toolbar-cs.patch1.03 KBzeta ζ

Comments

Status: Needs review » Needs work

The last submitted patch, toolbar-cs.patch, failed testing.

zeta ζ’s picture

zeta ζ commented
Status: Needs work » Needs review
StatusFileSize
new toolbar-cs.patch1 KB

slight omission…

idflood’s picture

idflood commented

Patch looks good for me. After applying it the toolbar toggle still behave as expected.

zeta ζ’s picture

zeta ζ commented

Hi idflood,

Thanks for testing this: Could you mark it RTBC? Being a code style issue, it’s simple enough to not need extensive testing. What you have done is perfectly adequate.

damien tournoud’s picture

damien tournoud commented
Status: Needs review » Reviewed & tested by the community

Sounds reasonable. This is a minor code clean-up.

dries’s picture

dries commented

This patch no longer applies. It needs a re-roll.

zeta ζ’s picture

zeta ζ commented
StatusFileSize
new toolbar-cs.patch1 KB

Re-roll…

Would you be interested in a coding-standards page, advising do not return from inside a control structure / one return at end of each function?

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed

Committed to HEAD. Thanks!

zeta ζ: That sounds like a good idea. Maybe coordinate either at http://groups.drupal.org/documentation-team or http://groups.drupal.org/coding-standards-and-best-practices on the best place for that?

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.