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

Issue 29603821: Noissue - Separated website-default utilities (Closed)

Created:
Nov. 10, 2017, 2:04 p.m. by juliandoucette
Modified:
Dec. 14, 2017, 6:22 p.m.
Reviewers:
ire
Base URL:
https://hg.adblockplus.org/website-defaults
Visibility:
Public.

Description

Noissue - Separated website-default utilities

Patch Set 1 #

Total comments: 15

Patch Set 2 : Rebased and re-separated #

Total comments: 10

Patch Set 3 : Consolidated utilities #

Total comments: 5

Patch Set 4 : Addressed comments #13 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+194 lines, -151 lines) Patch
M static/scss/_utilities.scss View 1 2 1 chunk +0 lines, -150 lines 0 comments Download
M static/scss/main.scss View 1 2 3 1 chunk +5 lines, -1 line 0 comments Download
A static/scss/utilities/_colors.scss View 1 2 1 chunk +39 lines, -0 lines 0 comments Download
A static/scss/utilities/_layout.scss View 1 2 1 chunk +50 lines, -0 lines 0 comments Download
A static/scss/utilities/_overrides.scss View 1 2 3 1 chunk +37 lines, -0 lines 0 comments Download
A static/scss/utilities/_text.scss View 1 2 3 1 chunk +26 lines, -0 lines 0 comments Download
A static/scss/utilities/_widths.scss View 1 2 1 chunk +37 lines, -0 lines 0 comments Download

Messages

Total messages: 16
juliandoucette
Nov. 10, 2017, 2:04 p.m. (2017-11-10 14:04:46 UTC) #1
juliandoucette
Please let em know what you think about the detail below. https://codereview.adblockplus.org/29603821/diff/29603822/static/scss/_utilities.scss File static/scss/_utilities.scss (right): ...
Nov. 10, 2017, 2:08 p.m. (2017-11-10 14:08:05 UTC) #2
ire
https://codereview.adblockplus.org/29603821/diff/29603822/static/scss/_utilities.scss File static/scss/_utilities.scss (right): https://codereview.adblockplus.org/29603821/diff/29603822/static/scss/_utilities.scss#newcode18 static/scss/_utilities.scss:18: * Utilities On 2017/11/10 14:08:04, juliandoucette wrote: > Detail: ...
Nov. 13, 2017, 8:33 a.m. (2017-11-13 08:33:40 UTC) #3
juliandoucette
https://codereview.adblockplus.org/29603821/diff/29603822/static/scss/_utilities.scss File static/scss/_utilities.scss (right): https://codereview.adblockplus.org/29603821/diff/29603822/static/scss/_utilities.scss#newcode18 static/scss/_utilities.scss:18: * Utilities On 2017/11/13 08:33:37, ire wrote: > If ...
Nov. 13, 2017, 12:33 p.m. (2017-11-13 12:33:00 UTC) #4
ire
https://codereview.adblockplus.org/29603821/diff/29603822/static/scss/_utilities.scss File static/scss/_utilities.scss (right): https://codereview.adblockplus.org/29603821/diff/29603822/static/scss/_utilities.scss#newcode18 static/scss/_utilities.scss:18: * Utilities On 2017/11/13 12:32:59, juliandoucette wrote: > On ...
Nov. 14, 2017, 7:14 a.m. (2017-11-14 07:14:51 UTC) #5
juliandoucette
https://codereview.adblockplus.org/29603821/diff/29603822/static/scss/utilities/_responsive-widths.scss File static/scss/utilities/_responsive-widths.scss (right): https://codereview.adblockplus.org/29603821/diff/29603822/static/scss/utilities/_responsive-widths.scss#newcode17 static/scss/utilities/_responsive-widths.scss:17: /** On 2017/11/14 07:14:51, ire wrote: > NIT: I ...
Nov. 17, 2017, 2:52 p.m. (2017-11-17 14:52:13 UTC) #6
juliandoucette
Note: It makes sense to resolve https://codereview.adblockplus.org/29612659/ before uploading a new PatchSet for this.
Nov. 27, 2017, 5:04 p.m. (2017-11-27 17:04:10 UTC) #7
ire
On 2017/11/27 17:04:10, juliandoucette wrote: > Note: It makes sense to resolve https://codereview.adblockplus.org/29612659/ > before ...
Nov. 27, 2017, 6:12 p.m. (2017-11-27 18:12:49 UTC) #8
juliandoucette
Updated.
Dec. 6, 2017, 3:56 p.m. (2017-12-06 15:56:05 UTC) #9
ire
Thanks, a couple of comments. https://codereview.adblockplus.org/29603821/diff/29631663/static/scss/_utilities.scss File static/scss/_utilities.scss (left): https://codereview.adblockplus.org/29603821/diff/29631663/static/scss/_utilities.scss#oldcode1 static/scss/_utilities.scss:1: // This file is ...
Dec. 7, 2017, 11:44 a.m. (2017-12-07 11:44:20 UTC) #10
juliandoucette
Thanks Ire! I'll update this again today. https://codereview.adblockplus.org/29603821/diff/29631663/static/scss/_utilities.scss File static/scss/_utilities.scss (left): https://codereview.adblockplus.org/29603821/diff/29631663/static/scss/_utilities.scss#oldcode1 static/scss/_utilities.scss:1: // This ...
Dec. 7, 2017, 12:33 p.m. (2017-12-07 12:33:58 UTC) #11
juliandoucette
https://codereview.adblockplus.org/29603821/diff/29631663/static/scss/main.scss File static/scss/main.scss (right): https://codereview.adblockplus.org/29603821/diff/29631663/static/scss/main.scss#newcode28 static/scss/main.scss:28: @import "utilities/fixed-widths.scss"; On 2017/12/07 11:44:20, ire wrote: > NIT: ...
Dec. 8, 2017, 4:11 p.m. (2017-12-08 16:11:07 UTC) #12
ire
LGTM + NITs below https://codereview.adblockplus.org/29603821/diff/29631663/static/scss/_utilities.scss File static/scss/_utilities.scss (left): https://codereview.adblockplus.org/29603821/diff/29631663/static/scss/_utilities.scss#oldcode1 static/scss/_utilities.scss:1: // This file is part ...
Dec. 11, 2017, 3:44 p.m. (2017-12-11 15:44:04 UTC) #13
juliandoucette
> It looks like it's still here. Is it actually an issue with rietveld? (Don't ...
Dec. 14, 2017, 1:37 p.m. (2017-12-14 13:37:08 UTC) #14
ire
LGTM again https://codereview.adblockplus.org/29603821/diff/29633749/static/scss/utilities/_text.scss File static/scss/utilities/_text.scss (right): https://codereview.adblockplus.org/29603821/diff/29633749/static/scss/utilities/_text.scss#newcode26 static/scss/utilities/_text.scss:26: .unstyled, On 2017/12/14 13:37:08, juliandoucette wrote: > ...
Dec. 14, 2017, 3:19 p.m. (2017-12-14 15:19:17 UTC) #15
juliandoucette
Dec. 14, 2017, 6:22 p.m. (2017-12-14 18:22:17 UTC) #16

Powered by Google App Engine
This is Rietveld