Skip to content
This repository has been archived by the owner on Nov 9, 2017. It is now read-only.

Vertical whitespace #30

Merged
merged 3 commits into from Jan 2, 2013
Merged

Vertical whitespace #30

merged 3 commits into from Jan 2, 2013

Conversation

jspahrsummers
Copy link
Contributor

This is strictly an aesthetic thing, but I generally find more vertical whitespace to aid readability, and after a control structure is one of the most effective places to do it.


```objc
if (shitIsBad) return;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line was just removed to contrast with the blank line below, not because it couldn't be here.

@ghost ghost assigned alanjrogers Dec 27, 2012
@alanjrogers
Copy link
Contributor

I'm pretty this is just articulating what we're all doing already, but I'll wait for a 👍 from @dannygreg and @joshaber before merging this.

@dannygreg
Copy link

I agree with this use of whitespace, but I'm not sure it really belongs in the guidelines. For me, it's not really a hard and fast rule, how you want to shape code like this. It is dealed with on a case by case basis.

@jspahrsummers
Copy link
Contributor Author

Yeah, that's fair enough. What I really want is:

  1. More liberal use of vertical whitespace.
  2. Some objective guideline to point to for PRs on our OSS repos, rather than just saying "this needs more whitespace."

Maybe there's some better way to encapsulate that, or maybe a guideline isn't actually necessary.

@joshaber
Copy link
Contributor

I'm 👍 on the style but I don't feel strongly as to whether it belongs in the conventions. Whatevs.

@alanjrogers
Copy link
Contributor

Perhaps we should just soften the language a little bit? Something like

We prefer a liberal use of vertical whitespace, for example, try to leave a blank line after a multi-line control structure if it's followed by another statement.

@jspahrsummers
Copy link
Contributor Author

@alanjrogers I like the general thrust of that. I riffed on it a little bit to create a more general rule – what do you guys think of this?

@alanjrogers
Copy link
Contributor

👍 from me.

:general: and :thrust: need emoji.

@dannygreg
Copy link

👍 to the thrust change.

chortle

alanjrogers pushed a commit that referenced this pull request Jan 2, 2013
@alanjrogers alanjrogers merged commit 4a5d1aa into master Jan 2, 2013
@alanjrogers alanjrogers deleted the control-structure-whitespace branch January 2, 2013 13:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
4 participants