Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Issue 29643562: Issue 5689 - Add rules for CSS declaration order to stylelint-config-eyeo (Closed)

Created:
Dec. 18, 2017, 9:14 a.m. by ire
Modified:
Feb. 8, 2018, 3:59 p.m.
Base URL:
https://hg.adblockplus.org/codingtools
Visibility:
Public.

Description

Issue 5689 - Add rules for CSS declaration order to stylelint-config-eyeo

Patch Set 1 #

Total comments: 3

Patch Set 2 : Rebase, ignore node_modules in any subdirectory #

Total comments: 2

Patch Set 3 : Fkexible order of properties #

Total comments: 6

Patch Set 4 : Fixed trailing whitespace errors #

Unified diffs Side-by-side diffs Delta from patch set Stats (+238 lines, -2 lines) Patch
M .hgignore View 1 1 chunk +1 line, -0 lines 0 comments Download
M stylelint-config-eyeo/README.md View 1 chunk +4 lines, -0 lines 0 comments Download
A stylelint-config-eyeo/css-properties-order.js View 1 2 3 1 chunk +223 lines, -0 lines 0 comments Download
M stylelint-config-eyeo/index.js View 1 2 chunks +8 lines, -1 line 0 comments Download
M stylelint-config-eyeo/package.json View 1 chunk +2 lines, -1 line 0 comments Download

Messages

Total messages: 19
ire
Dec. 18, 2017, 9:14 a.m. (2017-12-18 09:14:47 UTC) #1
ire
https://codereview.adblockplus.org/29643562/diff/29643563/stylelint-config-eyeo/css-properties-order.js File stylelint-config-eyeo/css-properties-order.js (right): https://codereview.adblockplus.org/29643562/diff/29643563/stylelint-config-eyeo/css-properties-order.js#newcode20 stylelint-config-eyeo/css-properties-order.js:20: module.exports = [ This was essentially copied from this ...
Dec. 18, 2017, 9:39 a.m. (2017-12-18 09:39:46 UTC) #2
juliandoucette
Relevant background information: https://codereview.adblockplus.org/29360001/diff/29362207/stylelintrc.json#newcode60
Dec. 18, 2017, 3:50 p.m. (2017-12-18 15:50:11 UTC) #3
ire
On 2017/12/18 15:50:11, juliandoucette wrote: > Relevant background information: > https://codereview.adblockplus.org/29360001/diff/29362207/stylelintrc.json#newcode60 AFAIK there is no ...
Dec. 18, 2017, 4:35 p.m. (2017-12-18 16:35:00 UTC) #4
juliandoucette
I'm torn because I think that linting individual property order may be too strict. Do ...
Dec. 18, 2017, 5:21 p.m. (2017-12-18 17:21:54 UTC) #5
ire
On 2017/12/18 17:21:54, juliandoucette wrote: > I'm torn because I think that linting individual property ...
Dec. 19, 2017, 10:14 a.m. (2017-12-19 10:14:57 UTC) #6
juliandoucette
What do you think Thomas?
Jan. 4, 2018, 3:26 p.m. (2018-01-04 15:26:35 UTC) #7
Thomas Greiner
On 2018/01/04 15:26:35, juliandoucette wrote: > What do you think Thomas? 1) "declaration-block-properties-order" seems to ...
Jan. 8, 2018, 2:40 p.m. (2018-01-08 14:40:34 UTC) #8
ire
Thanks Thomas! On 2018/01/08 14:40:34, Thomas Greiner wrote: > 2) As I mentioned in the ...
Jan. 9, 2018, 8:16 a.m. (2018-01-09 08:16:30 UTC) #9
Thomas Greiner
Please ignore the first point in my previous comment. I don't know why I mentioned ...
Jan. 9, 2018, 2:15 p.m. (2018-01-09 14:15:05 UTC) #10
ire
On 2018/01/09 14:15:05, Thomas Greiner wrote: > Please ignore the first point in my previous ...
Jan. 10, 2018, 10:18 a.m. (2018-01-10 10:18:51 UTC) #11
juliandoucette
I'm ok with trying this. LGTM. https://codereview.adblockplus.org/29643562/diff/29661558/stylelint-config-eyeo/css-properties-order.js File stylelint-config-eyeo/css-properties-order.js (right): https://codereview.adblockplus.org/29643562/diff/29661558/stylelint-config-eyeo/css-properties-order.js#newcode21 stylelint-config-eyeo/css-properties-order.js:21: // Display NIT: ...
Jan. 15, 2018, 5:51 p.m. (2018-01-15 17:51:58 UTC) #12
ire
Thanks Julian! https://codereview.adblockplus.org/29643562/diff/29661558/stylelint-config-eyeo/css-properties-order.js File stylelint-config-eyeo/css-properties-order.js (right): https://codereview.adblockplus.org/29643562/diff/29661558/stylelint-config-eyeo/css-properties-order.js#newcode21 stylelint-config-eyeo/css-properties-order.js:21: // Display On 2018/01/15 17:51:58, juliandoucette wrote: ...
Jan. 22, 2018, 8:52 a.m. (2018-01-22 08:52:55 UTC) #13
ire
@Thomas I've change the properties ordering to flexible
Jan. 23, 2018, 1:31 p.m. (2018-01-23 13:31:00 UTC) #14
Thomas Greiner
On 2018/01/23 13:31:00, ire wrote: > @Thomas I've change the properties ordering to flexible Thanks, ...
Jan. 23, 2018, 1:40 p.m. (2018-01-23 13:40:47 UTC) #15
ire
@Tristan or @Sebastian please could one of you take a look at this? I've gotten ...
Jan. 24, 2018, 9:51 a.m. (2018-01-24 09:51:59 UTC) #16
tlucas
Hey Ire, sorry for the late reply. I only noted some whitespace-errors, please see below. ...
Feb. 8, 2018, 11:51 a.m. (2018-02-08 11:51:48 UTC) #17
ire
On 2018/02/08 11:51:48, tlucas wrote: > Hey Ire, > > sorry for the late reply. ...
Feb. 8, 2018, 2:12 p.m. (2018-02-08 14:12:08 UTC) #18
tlucas
Feb. 8, 2018, 2:32 p.m. (2018-02-08 14:32:16 UTC) #19
On 2018/02/08 14:12:08, ire wrote:
> On 2018/02/08 11:51:48, tlucas wrote:
> > Hey Ire,
> > 
> > sorry for the late reply.
> > I only noted some whitespace-errors, please see below.
> > 
> > Cheers!
> 
> No problem! I've fixed the errors
> 
>
https://codereview.adblockplus.org/29643562/diff/29677653/stylelint-config-ey...
> File stylelint-config-eyeo/css-properties-order.js (right):
> 
>
https://codereview.adblockplus.org/29643562/diff/29677653/stylelint-config-ey...
> stylelint-config-eyeo/css-properties-order.js:40: 
> On 2018/02/08 11:51:47, tlucas wrote:
> > This line triggers a "trailing whitespace".
> 
> Done.
> 
>
https://codereview.adblockplus.org/29643562/diff/29677653/stylelint-config-ey...
> stylelint-config-eyeo/css-properties-order.js:113: 
> On 2018/02/08 11:51:47, tlucas wrote:
> > This line triggers a "trailing whitespace".
> 
> Done.
> 
>
https://codereview.adblockplus.org/29643562/diff/29677653/stylelint-config-ey...
> stylelint-config-eyeo/css-properties-order.js:182: 
> On 2018/02/08 11:51:47, tlucas wrote:
> > This line triggers a "trailing whitespace".
> 
> Done.

LGTM.

Export me the patch and i'll push it :)

Powered by Google App Engine
This is Rietveld