cleanup the style guide #2415
cleanup the style guide #2415
Conversation
yjbanov
commented
Feb 22, 2016
yjbanov
commented
- do not repeat the Dart style guide
- use DO/DON'T/AVOID/PREVER headings similar to Effective Dart's
- consistent code example formatting
- consistent GOOD/BAD example formatting
- consistent heading syntax
- consistent line length
- more code samples
|
Once this lands, we should also consider moving it to flutter/flutter or flutter/website. It's really strangely placed atm. |
| @@ -1,5 +1,4 @@ | |||
| Flutter Style Guide | |||
| =================== | |||
| # Flutter Style Guide | |||
There was a problem hiding this comment.
Prefer === because it makes it more readable in text mode.
There was a problem hiding this comment.
Me too, but I don't know if this style supports anything beyond 2nd-level headings, so I decided to switch everything to # for consistency sake.
There was a problem hiding this comment.
PREFER readability over consistency. :-)
There was a problem hiding this comment.
Consistency contributes to readability big time. But we're talking about a couple headlines here, so I'll revert the change.
|
We should probably kill the stubs for Java, Objective-C and C++ at the bottom, if we want to move it to flutter/flutter. |
|
|
||
| Avoid checking in commented-out code. It will bitrot too fast to be | ||
| useful, and will confuse people maintaining the code. | ||
| ### AVOID checking in commented-out code |
There was a problem hiding this comment.
The all-caps "AVOID" and "DON'T" and so on is pretty ugly. Can we do something more typographically pleasing?
In general having headings makes sense (the existing style guide is a wall of text with no headings organising it), but I'm not sure having the rules in the headings make sense. Why, for example, does "AVOID checking in comments that ask questions" have a heading, but "If commenting on a workaround due to a bug, also leave a link to the bug and a TODO to clean it up when the bug is fixed." not? Why is the heading "AVOID checking in comments that ask questions" rather than "DO find the answers to the questions, or describe the confusion, including references to where you found answers"?
Should the headings be more things like "Comments", "Line breaking" etc?
There was a problem hiding this comment.
The answer to everything is: be consistent with the standard style guide. This style guide is a delta on top of the standard Dart guide, which we recommend that people read. Therefore, it makes a lot of sense to preserve the same style. Some additional points:
Why rules in the headings
- they make it easy to quickly scan the guide and familiarizing yourself with the main points
- they make it easy to copy and paste guidelines into a code review comment
What's with the choice of headlines
Like in the standard guide, each headline attempts to make the most important succinctly. The paragraphs then expand on the point, give examples, and mention asides. We can certainly switch things up as necessary. I by no means insist that my choice of headlines is the most optimal. I'm simply bootstrapping an idea.
There was a problem hiding this comment.
I think the Dart style guide using capital letters in the headings is pretty ugly, but the way the Dart style guide is organised they first have headings for each topic, and then have headings that barely look like more than emphasised paragraphs.
I think we could find a middle ground where we have topic headings, then subheadings for each major guideline, but where the guidelines are written as full sentences with normal capitalisation (rather than shoehorning them into this "DO that" "PREFER this" style).
|
Overall this is a good improvement but I'm not a fan of the heading style. |
- do not repeat the Dart style guide - use DO/DON'T/AVOID/PREVER headings similar to Effective Dart's - consistent code example formatting - consistent GOOD/BAD example formatting - consistent heading syntax - consistent line length
|
Alright, I removed the all-caps DOs and DON'Ts, and added sub-sections. I tried to keep "Do", "Don't", "Prefer", etc where a clear sentence can be formed, and deviated where shoehorning a phrase would be necessary. Having the first word be clear about whether you are encouraged to or discouraged from doing something is useful. I also removed the speculative sections for other languages. It's easy to add them back. Submitting. We can iterate on it later. |
|
LGTM. (As a general rule please wait until you have an explicit LGTM before checking something in -- in this case, for example, I hadn't finished reviewing it and was waiting for the headings to get resolved before going through it again more thoroughly.) |
|
Sorry, I read your "Overall this is a good improvement" as LGTM. What do you think of adding a label "lgtm" to the engine and flutter repos? I've used it in the past and it's a much more concrete way of LGTM'ing PRs. |
…er#2415… (flutter#24331) This reverts commit 2e10a97.
