Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(236)

Issue 29677659: Issue #6303 - Introduce basic foundation for automation in adblockplusui (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 year, 7 months ago by a.giammarchi
Modified:
1 year, 6 months ago
Visibility:
Public.

Description

Issue #6303 - Introduce basic foundation for automation in adblockplusui

Patch Set 1 #

Total comments: 21

Patch Set 2 : applied required changes #

Patch Set 3 : #

Total comments: 6

Patch Set 4 : #

Patch Set 5 : #

Total comments: 7

Patch Set 6 : #

Patch Set 7 : #

Patch Set 8 : #

Patch Set 9 : #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+56 lines, -47 lines) Patch
A .hgignore View 1 chunk +1 line, -0 lines 0 comments Download
A .npmrc View 1 chunk +1 line, -0 lines 0 comments Download
M README.md View 1 2 3 4 5 6 7 8 3 chunks +38 lines, -17 lines 2 comments Download
A package.json View 1 2 3 4 5 6 7 1 chunk +16 lines, -0 lines 0 comments Download
R test_server.py View 1 1 chunk +0 lines, -30 lines 0 comments Download

Messages

Total messages: 32
a.giammarchi
I have added http-server to the list of dependencies for the simple reason that is ...
1 year, 7 months ago (2018-01-23 14:50:17 UTC) #1
Thomas Greiner
Looking good overall. One comment I'd like to highlight is the one about dependencies. I ...
1 year, 7 months ago (2018-01-23 19:22:36 UTC) #2
a.giammarchi
answered to all the things https://codereview.adblockplus.org/29677659/diff/29677660/README.md File README.md (right): https://codereview.adblockplus.org/29677659/diff/29677660/README.md#newcode16 README.md:16: You need Python version ...
1 year, 7 months ago (2018-01-24 10:52:37 UTC) #3
Thomas Greiner
https://codereview.adblockplus.org/29677659/diff/29677660/README.md File README.md (right): https://codereview.adblockplus.org/29677659/diff/29677660/README.md#newcode16 README.md:16: You need Python version 2 to ensure a successful ...
1 year, 7 months ago (2018-01-24 11:56:33 UTC) #4
a.giammarchi
applied all required changes. The stylelint-config-eyeo entry in package json still needs for stylelint-config-eyeo to ...
1 year, 7 months ago (2018-01-24 13:47:24 UTC) #5
Thomas Greiner
Great, let's wait until stylelint-config-eyeo is published then. Only added one last comment in regards ...
1 year, 7 months ago (2018-01-24 15:40:36 UTC) #6
a.giammarchi
On 2018/01/24 15:40:36, Thomas Greiner wrote: > Great, let's wait until stylelint-config-eyeo is published then. ...
1 year, 7 months ago (2018-01-24 15:45:35 UTC) #7
a.giammarchi
answered
1 year, 7 months ago (2018-01-24 15:45:49 UTC) #8
Thomas Greiner
On 2018/01/24 15:45:35, a.giammarchi wrote: > Why are we waiting for the stylelint-config-eyeo package to ...
1 year, 6 months ago (2018-01-24 16:08:14 UTC) #9
a.giammarchi
On 2018/01/24 16:08:14, Thomas Greiner wrote: > On 2018/01/24 15:45:35, a.giammarchi wrote: > > Why ...
1 year, 6 months ago (2018-01-24 16:24:02 UTC) #10
Thomas Greiner
On 2018/01/24 16:24:02, a.giammarchi wrote: > I don't think dropping changes to the README and ...
1 year, 6 months ago (2018-01-24 16:42:57 UTC) #11
a.giammarchi
this should be clean enough and ready to go. Please let me know if there's ...
1 year, 6 months ago (2018-02-06 15:54:40 UTC) #12
a.giammarchi
ready to go
1 year, 6 months ago (2018-02-06 15:54:51 UTC) #13
saroyanm
Just a small things/nits I've noticed and 1 question. https://codereview.adblockplus.org/29677659/diff/29690905/.stylelintrc.json File .stylelintrc.json (right): https://codereview.adblockplus.org/29677659/diff/29690905/.stylelintrc.json#newcode2 .stylelintrc.json:2: ...
1 year, 6 months ago (2018-02-06 17:33:24 UTC) #14
Thomas Greiner
https://codereview.adblockplus.org/29677659/diff/29690905/README.md File README.md (right): https://codereview.adblockplus.org/29677659/diff/29690905/README.md#newcode63 README.md:63: You can also run only JS via `npm run ...
1 year, 6 months ago (2018-02-06 19:08:48 UTC) #15
saroyanm
https://codereview.adblockplus.org/29677659/diff/29690905/package.json File package.json (right): https://codereview.adblockplus.org/29677659/diff/29690905/package.json#newcode9 package.json:9: "start": "http-server $1" On 2018/02/06 19:08:48, Thomas Greiner wrote: ...
1 year, 6 months ago (2018-02-07 09:07:54 UTC) #16
a.giammarchi
On 2018/02/07 09:07:54, saroyanm wrote: > https://codereview.adblockplus.org/29677659/diff/29690905/package.json > File package.json (right): > > https://codereview.adblockplus.org/29677659/diff/29690905/package.json#newcode9 > ...
1 year, 6 months ago (2018-02-07 10:02:16 UTC) #17
a.giammarchi
README changes applied + npm start accepts 0 to N extra arguments when/if needed to ...
1 year, 6 months ago (2018-02-07 10:05:36 UTC) #18
saroyanm
https://codereview.adblockplus.org/29677659/diff/29691567/README.md File README.md (right): https://codereview.adblockplus.org/29677659/diff/29691567/README.md#newcode63 README.md:63: You can also run only JS via `npm run ...
1 year, 6 months ago (2018-02-07 11:48:27 UTC) #19
a.giammarchi
On 2018/02/07 11:48:27, saroyanm wrote: > https://codereview.adblockplus.org/29677659/diff/29691567/README.md > File README.md (right): > > https://codereview.adblockplus.org/29677659/diff/29691567/README.md#newcode63 > ...
1 year, 6 months ago (2018-02-07 12:06:06 UTC) #20
saroyanm
https://codereview.adblockplus.org/29677659/diff/29691567/package.json File package.json (right): https://codereview.adblockplus.org/29677659/diff/29691567/package.json#newcode9 package.json:9: "start": "http-server \"${@:2}\"" On 2018/02/07 11:48:26, saroyanm wrote: > ...
1 year, 6 months ago (2018-02-07 12:08:16 UTC) #21
a.giammarchi
On 2018/02/07 12:08:16, saroyanm wrote: > https://codereview.adblockplus.org/29677659/diff/29691567/package.json > File package.json (right): > > https://codereview.adblockplus.org/29677659/diff/29691567/package.json#newcode9 > ...
1 year, 6 months ago (2018-02-07 12:17:28 UTC) #22
saroyanm
Sorry were having Technical issues after update, had to update bash manually, but still same ...
1 year, 6 months ago (2018-02-07 14:02:04 UTC) #23
a.giammarchi
On 2018/02/07 14:02:04, saroyanm wrote: > Sorry were having Technical issues after update, had to ...
1 year, 6 months ago (2018-02-07 14:14:00 UTC) #24
saroyanm
https://codereview.adblockplus.org/29677659/diff/29691567/package.json File package.json (right): https://codereview.adblockplus.org/29677659/diff/29691567/package.json#newcode9 package.json:9: "start": "http-server \"${@:2}\"" On 2018/02/07 14:02:04, saroyanm wrote: > ...
1 year, 6 months ago (2018-02-07 14:21:41 UTC) #25
a.giammarchi
https://codereview.adblockplus.org/29677659/diff/29691567/package.json File package.json (right): https://codereview.adblockplus.org/29677659/diff/29691567/package.json#newcode9 package.json:9: "start": "http-server \"${@:2}\"" On 2018/02/07 14:21:41, saroyanm wrote: > ...
1 year, 6 months ago (2018-02-07 14:30:04 UTC) #26
saroyanm
https://codereview.adblockplus.org/29677659/diff/29691567/package.json File package.json (right): https://codereview.adblockplus.org/29677659/diff/29691567/package.json#newcode9 package.json:9: "start": "http-server \"${@:2}\"" On 2018/02/07 14:30:04, a.giammarchi wrote: > ...
1 year, 6 months ago (2018-02-07 14:31:43 UTC) #27
a.giammarchi
ready for review
1 year, 6 months ago (2018-02-08 09:44:47 UTC) #28
saroyanm
All looks great to me, Happy that we are finally moving forward with the automation. ...
1 year, 6 months ago (2018-02-08 14:33:13 UTC) #29
Thomas Greiner
LGTM I added a suggestion so I'll leave it up to you whether or not ...
1 year, 6 months ago (2018-02-12 13:01:44 UTC) #30
a.giammarchi
answered https://codereview.adblockplus.org/29677659/diff/29691610/README.md File README.md (right): https://codereview.adblockplus.org/29677659/diff/29691610/README.md#newcode12 README.md:12: Both [python 2](https://www.python.org/downloads/) and [node](https://nodejs.org/en/), as well as ...
1 year, 6 months ago (2018-02-12 14:52:55 UTC) #31
Thomas Greiner
1 year, 6 months ago (2018-02-13 16:35:37 UTC) #32
Message was sent while issue was closed.
On 2018/02/12 14:52:55, a.giammarchi wrote:
> agreed but this has been already merged so I'll keep that in mind for the next
> time. Does it sound reasonable?

Yep, that's fine.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 87257f5