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.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Status: Needs review » Needs work

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

zeta ζ’s picture

Status: Needs work » Needs review
FileSize
1 KB

slight omission…

idflood’s picture

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

zeta ζ’s picture

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

Status: Needs review » Reviewed & tested by the community

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

Dries’s picture

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

zeta ζ’s picture

FileSize
1 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

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.