#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):

https://codereview.adblockplus.org/29578555

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."

Change History (10)

comment:1 Changed 21 months ago by ire

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."

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.

comment:2 Changed 21 months ago by ire

  • Blocking 5109 added

comment:3 Changed 20 months ago by ire

@all Any reason why this shouldn't be marked ready yet?

comment:4 follow-up: Changed 20 months ago 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).

Last edited 20 months ago by juliandoucette (previous) (diff)

comment:5 in reply to: ↑ 4 ; follow-up: Changed 20 months ago 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 20 months ago 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 20 months ago by ire

  • Owner set to ire

comment:8 Changed 20 months ago by ire

  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

comment:9 Changed 20 months ago by abpbot

A commit referencing this issue has landed:
Issue 5676 - Relax CSS coding style

comment:10 Changed 20 months ago by ire

  • Resolution set to fixed
  • Status changed from reviewing to closed
Note: See TracTickets for help on using tickets.