Opened on 09/14/2017 at 10:44:13 AM
Closed on 10/16/2017 at 09:19:18 AM
#5676 closed change (fixed)
Relax CSS coding style
| Reported by: | greiner | Assignee: | ire |
|---|---|---|---|
| Priority: | P3 | Milestone: | |
| Module: | Websites | Keywords: | |
| Cc: | ire, saroyanm, juliandoucette, erikvold, greiner | Blocked By: | |
| Blocking: | #5109 | Platform: | Unknown / Cross platform |
| Ready: | yes | Confidential: | no |
| Tester: | Unknown | Verified working: | yes |
| Review URL(s): | |||
Description
Background
In #5109 we'll be introducing linting for our CSS code. As part of the code review we've questioned the legitimacy of certain parts of the coding style and would now like to relax those rules.
What to change
Modify https://adblockplus.org/coding-style#html-css
- Add "CSS declaration blocks with a single selector and a single rule can be written in a single line."
- Remove "CSS number values should specify units where possible."
Attachments (0)
Change History (10)
comment:1 Changed on 09/15/2017 at 07:57:21 AM by ire
comment:2 Changed on 09/15/2017 at 08:58:09 AM by ire
- Blocking 5109 added
comment:3 Changed on 10/13/2017 at 09:59:39 AM by ire
@all Any reason why this shouldn't be marked ready yet?
comment:4 follow-up: ↓ 5 Changed on 10/13/2017 at 02:30:56 PM by juliandoucette
- Cc greiner added
- Priority changed from Unknown to P3
Add "CSS declaration blocks with a single selector and a single rule can be written in a single line."
+1
Remove "CSS number values should specify units where possible."
+1
Should this rule be removed? Or just altered to allow for unit-less 0 values, e.g: "CSS number values should specify units where possible. But omit unit specification after “0” values, unless required." Either way is fine by me
I think that the real question here is "Do we want to allow zeros with units?" If not, then this rule should be "Do not add units to zero values" (or similar). Else I think that we can just remove this rule.
My vote goes towards (strictly) no units on zeros (they add unnecessary file-size).
comment:5 in reply to: ↑ 4 ; follow-up: ↓ 6 Changed on 10/13/2017 at 03:02:22 PM by greiner
Replying to juliandoucette:
I think that the real question here is "Do we want to allow zeros with units?" If not, then this rule should be "Do not add units to zero values" (or similar). Else I think that we can just remove this rule.
Note that by removing the existing rule it means that we'll automatically fall back to what's specified in Google's coding style which already states Omit unit specification after “0” values, unless required.
comment:6 in reply to: ↑ 5 Changed on 10/13/2017 at 03:17:15 PM by juliandoucette
- Ready set
- Verified working set
Replying to greiner:
Note that by removing the existing rule it means that we'll automatically fall back to what's specified in Google's coding style which already states Omit unit specification after “0” values, unless required.
Ack.
comment:7 Changed on 10/16/2017 at 08:04:17 AM by ire
- Owner set to ire
comment:8 Changed on 10/16/2017 at 08:09:53 AM by ire
- Review URL(s) modified (diff)
- Status changed from new to reviewing
comment:9 Changed on 10/16/2017 at 09:18:31 AM by abpbot
A commit referencing this issue has landed:
Issue 5676 - Relax CSS coding style
comment:10 Changed on 10/16/2017 at 09:19:18 AM by ire
- Resolution set to fixed
- Status changed from reviewing to closed
+1
Should this rule be removed? Or just altered to allow for unit-less 0 values, e.g: "CSS number values should specify units where possible. But omit unit specification after “0” values, unless required." Either way is fine by me.