Opened 9 months ago

Closed 3 days 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 9 months 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 9 months ago by juliandoucette

  • Cc fhd added

comment:3 Changed 8 months ago by juliandoucette

  • Priority changed from P2 to P3

comment:4 Changed 5 months 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 5 months 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: (in order of precedence)
      • codestyle recommendations
      • existing code
      • personal preferences
    • I didn't document why I chose each option
    • I seem to have taken a misguided "stricter is better" approach

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

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

comment:6 in reply to: ↑ 5 Changed 5 months 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 3 months ago by juliandoucette

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

comment:8 Changed 3 months ago by ire

  • Owner set to ire

comment:9 Changed 3 months ago by ire

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

comment:10 Changed 3 months ago by ire

  • Blocking 5689 added

comment:11 Changed 3 months ago by ire

  • Blocked By 5676 added

comment:12 follow-up: Changed 4 weeks ago by juliandoucette

It seems you forgot to commit/push this ire.

comment:13 in reply to: ↑ 12 Changed 3 weeks 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 3 days ago by abpbot

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

comment:15 Changed 3 days ago by ire

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