Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Formatting and consistency changes #1112

Closed
wants to merge 5 commits into from
Closed

Formatting and consistency changes #1112

wants to merge 5 commits into from

Conversation

necolas
Copy link
Member

@necolas necolas commented Jun 1, 2012

Up for review.

  • Consistent use of quotes in HTML
  • Consistent color values in CSS
  • Multi-line CSS and JS
  • 4 spaces for indentation
  • Always use closing tags
  • Use common DOCTYPE capitalization
  • New lines at the end of files

Steps toward reducing the number of repeat issues we get around many of these things and improve diff/cat readability.

Also aiming to work towards ensuring that HTML5 Boilerplate can continue to be used with "no surprises" by web app/site developers and more specialized web app boilerplates/toolkits. I think the future of the project is as much to provide a starting point for web app boilerplate/toolkit developers as it is to be a resource for beginners to learn from. In keeping with that, I'm open to any adaptation of the project that can help make it even more useful for those developers.

Thanks.

Move to a more sensible and readable multi-line CSS format. Better for
diffs too.
Four spaces is generally the most common indentation level, and
particularly common for CSS.

Capitalized the DOCTYPE both to avoid issues being created about it, and
to adhere to a more common pattern. Added closing tags to the 404 for
the same reasons.
Make the file readable.
@necolas necolas mentioned this pull request Jun 1, 2012
@nimbupani
Copy link
Member

👍 to everything but not capitalize DOCTYPE. PLZ PLZ PLZ

@davidmurdoch
Copy link

+1 to everything but -Infinity for capitalized doctype.

@paulirish
Copy link
Member

Also aiming to work towards ensuring that HTML5 Boilerplate can continue to be used with "no surprises" by web app/site developers and more specialized web app boilerplates/toolkits. I think the future of the project is ... to provide a starting point for web app boilerplate/toolkit developers

👍

@mathiasbynens
Copy link
Member

LGTM.

Some of these decisions do not match my personal coding style (tabs instead of spaces? FFFFUUUUU), but I’ll happily ignore that fact for the sake of project consistency.

@mathiasbynens
Copy link
Member

One thing that’s probably a mistake though — what’s with the double newlines at EOF? Seems like one is more than enough, no?

@rutger1140
Copy link
Contributor

👍 Readable code; a measure of the developers skill.

One question though: What are the arguments that I should be using 4 spaces in stead of 2, besides personal coding-style preference?

I prefer to keep my CSS within the 80 columns. Any remote terminal I connect to, to apply some patch in VIM, remains properly usable. (Of course this is not always feasible)

When using a CSS preprocessor, indenting each nested set with 4 spaces increases the column count tremendously.

@ghost
Copy link

ghost commented Jun 2, 2012

He said "tabs instead of spaces" not "4 vs 2 spaces". However I could be misinterpreting what he meant.

@necolas
Copy link
Member Author

necolas commented Jun 2, 2012

One thing that’s probably a mistake though — what’s with the double newlines at EOF? Seems like one is more than enough, no?

Where are the double newlines at? Related info for the curious: http://stackoverflow.com/q/729692/96656

What are the arguments that I should be using 4 spaces instead of 2, besides personal coding-style preference?

None of which I know. 4 spaces appears to be the most common convention across various languages (pretty sure I saw a FE survey a few years ago that showed 4 to be the most commonly used too). It's pretty readable.

When using a CSS preprocessor, indenting each nested set with 4 spaces increases the column count tremendously.

You shouldn't really be nesting to that extreme. 1 or 2 level nesting max :)

@AD7six
Copy link
Member

AD7six commented Jun 2, 2012

Where are the double newlines at? Related info for the curious: http://stackoverflow.com/q/729692/96656

The blank line at the end of all files is not necessary (and perhaps considered untidy). It's not the same thing as "files should end with a new line" - which is editor-config, not user, dependent.

This https://github.com/h5bp/html5-boilerplate/pull/1112/files#L0L42 did not end with a new line previously - the git-diff-comment is referring to the previous line. All files in this commit end with a blank line - which is itself terminated with a newline char.

In pseudo-detail

Bad (typical with textmate IME)

File contents\n
Last Line

Good/expected

File contents\n
Last Line\n

This commit

File contents\n
Last Line\n
\n

@necolas
Copy link
Member Author

necolas commented Jun 2, 2012

Thanks for the explanation! Was wondering why I didn't get the newline warning all the time - Vim obviously gets it right whereas files from other editors sometimes don't. I'll correct this.

@necolas
Copy link
Member Author

necolas commented Jun 2, 2012

Cleaned up and applied to master in 7a2f9dc

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants