Opened 2 years ago

Closed 2 years ago

#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 2 years 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 2 years ago by ire

  • Blocking 5109 added

comment:3 Changed 2 years ago by ire

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

comment:4 follow-up: Changed 2 years 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 2 years ago by juliandoucette (previous) (diff)

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

  • Owner set to ire

comment:8 Changed 2 years ago by ire

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

comment:9 Changed 2 years ago by abpbot

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

comment:10 Changed 2 years ago by ire

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