Opened 3 years ago

Closed 2 years ago

#5109 closed change (fixed)

Create stylelintrc for websites and ui modules

Reported by: juliandoucette Assignee: ire
Priority: P2 Milestone:
Module: Websites Keywords:
Cc: saroyanm, greiner, martin, p.pastourmatzis, erick, fhd Blocked By: #5676
Blocking: #5689 Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

https://codereview.adblockplus.org/29541680/

Description

Background

Stylelint is a mighty, modern CSS linter that helps you enforce consistent conventions and avoid errors in your stylesheets.

What to change

Create a stylelintrc that matches to our coding-style guidelines for CSS as closely as possible.

Change History (15)

comment:1 Changed 3 years ago by juliandoucette

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

Prioritized because this can be useful for the options page, help center, and onboarding new websites/ui developers soon.

comment:2 Changed 3 years ago by juliandoucette

  • Cc fhd added

comment:3 Changed 3 years ago by juliandoucette

  • Priority changed from P2 to P3

comment:4 Changed 3 years ago by erikvold

Hoping to get this bumped up in priority, I'd like to use the results for the flattr ext.

comment:5 follow-up: Changed 3 years ago by juliandoucette

  • Priority changed from P3 to P2

hoping to get this bumped up in priority, I'd like to use the results for the flattr ext.

Agreed. You are welcome to take it on if you'd like.

Unfortunately:

  • Google's code style has changed (since my last patchset)
  • Stylelint has changed (since my last patchset)
  • The options available at the time didn't fit our recommendations well (IIRC, in my last patchset)
    • I chose the options that I did based on our recommendations, our existing code, and my personal preferences (but I didn't document why I chose each property)

Therefore, it's probably going to be more efficient if we start over.

Version 1, edited 3 years ago by juliandoucette (previous) (next) (diff)

comment:6 in reply to: ↑ 5 Changed 3 years ago by erikvold

Replying to juliandoucette:

hoping to get this bumped up in priority, I'd like to use the results for the flattr ext.

Agreed. You are welcome to take it on if you'd like.
...
Therefore, it's probably going to be more efficient if we start over.

I've started a pull request to use stylelint for Flattr, https://github.com/flattr/rising-tide/pull/204

Maybe someone could review that and see if it's conforming with eyeo coding style? I think Thomas wanted to resolve this issue first then handle Flattr, so this would be kind of the opposite direction unless someone wants to pick up this issue.

I'm not sure I'm the best person to implement this for the eyeo wide configuration; I constantly break our coding style, and it would probably involve testing the configuration on a few of our projects.

It might be easier to implement on one project first then clone the config to a few projects until we are ready to merge the configs, it's a little less organized implementation, but it means we can start using stylelint sooner rather than later.

comment:7 Changed 2 years ago by juliandoucette

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

comment:8 Changed 2 years ago by ire

  • Owner set to ire

comment:9 Changed 2 years ago by ire

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

comment:10 Changed 2 years ago by ire

  • Blocking 5689 added

comment:11 Changed 2 years ago by ire

  • Blocked By 5676 added

comment:12 follow-up: Changed 2 years ago by juliandoucette

It seems you forgot to commit/push this ire.

comment:13 in reply to: ↑ 12 Changed 2 years ago by ire

Replying to juliandoucette:

It seems you forgot to commit/push this ire.

No, I just don't have push access to the codingtools repository. I've emailed to sort this out so it should be committed soon.

comment:14 Changed 2 years ago by abpbot

A commit referencing this issue has landed:
Issue 5109 - Create stylelintrc for websites and ui modules

comment:15 Changed 2 years ago by ire

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