Opened on 04/05/2017 at 11:55:54 AM

Closed on 12/14/2017 at 02:49:03 PM

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

Attachments (0)

Change History (15)

comment:1 Changed on 04/05/2017 at 11:56:51 AM 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 on 04/05/2017 at 11:58:13 AM by juliandoucette

  • Cc fhd added

comment:3 Changed on 04/26/2017 at 09:42:53 AM by juliandoucette

  • Priority changed from P2 to P3

comment:4 Changed on 07/20/2017 at 08:38:25 PM 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 on 07/23/2017 at 03:08:38 PM 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 on 07/24/2017 at 04:14:16 PM by juliandoucette

comment:6 in reply to: ↑ 5 Changed on 07/24/2017 at 06:48:12 PM 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 on 09/04/2017 at 05:28:27 PM by juliandoucette

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

comment:8 Changed on 09/11/2017 at 09:27:06 AM by ire

  • Owner set to ire

comment:9 Changed on 09/11/2017 at 09:27:15 AM by ire

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

comment:10 Changed on 09/15/2017 at 08:02:47 AM by ire

  • Blocking 5689 added

comment:11 Changed on 09/15/2017 at 08:58:09 AM by ire

  • Blocked By 5676 added

comment:12 follow-up: Changed on 11/22/2017 at 03:19:09 PM by juliandoucette

It seems you forgot to commit/push this ire.

comment:13 in reply to: ↑ 12 Changed on 11/25/2017 at 01:11:40 PM 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 on 12/14/2017 at 02:47:59 PM by abpbot

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

comment:15 Changed on 12/14/2017 at 02:49:03 PM by ire

  • Resolution set to fixed
  • Status changed from reviewing to closed

Add Comment

Modify Ticket

Change Properties
Action
as closed .
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from ire.
 
Note: See TracTickets for help on using tickets.