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

Issue 29321041: Issue 2707 - Support start/end notification parameters (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 years, 2 months ago by Felix Dahlke
Modified:
4 years, 1 month ago
Reviewers:
Sebastian Noack
CC:
Wladimir Palant
Visibility:
Public.

Description

Issue 2707 - Support start/end notification parameters

Patch Set 1 #

Total comments: 9

Patch Set 2 : Fix import order #

Patch Set 3 : Change date format #

Unified diffs Side-by-side diffs Delta from patch set Stats (+118 lines, -0 lines) Patch
M sitescripts/notifications/parser.py View 1 2 3 chunks +8 lines, -0 lines 0 comments Download
A sitescripts/notifications/test/parser.py View 1 2 1 chunk +110 lines, -0 lines 0 comments Download

Messages

Total messages: 8
Felix Dahlke
Made most sense to me to check for start/end in the parser (just like inactive), ...
4 years, 2 months ago (2015-06-23 15:21:16 UTC) #1
Sebastian Noack
https://codereview.adblockplus.org/29321041/diff/29321042/sitescripts/notifications/parser.py File sitescripts/notifications/parser.py (right): https://codereview.adblockplus.org/29321041/diff/29321042/sitescripts/notifications/parser.py#newcode20 sitescripts/notifications/parser.py:20: import dateutil.parser Nit: Third party module imports go below ...
4 years, 2 months ago (2015-06-23 15:41:37 UTC) #2
Sebastian Noack
https://codereview.adblockplus.org/29321041/diff/29321042/sitescripts/notifications/test/parser.py File sitescripts/notifications/test/parser.py (right): https://codereview.adblockplus.org/29321041/diff/29321042/sitescripts/notifications/test/parser.py#newcode20 sitescripts/notifications/test/parser.py:20: import mock Nit: Third party module imports go below ...
4 years, 2 months ago (2015-06-23 15:42:44 UTC) #3
Felix Dahlke
New patch set up that fixes the import order. https://codereview.adblockplus.org/29321041/diff/29321042/sitescripts/notifications/parser.py File sitescripts/notifications/parser.py (right): https://codereview.adblockplus.org/29321041/diff/29321042/sitescripts/notifications/parser.py#newcode20 sitescripts/notifications/parser.py:20: ...
4 years, 2 months ago (2015-06-23 15:51:44 UTC) #4
Sebastian Noack
https://codereview.adblockplus.org/29321041/diff/29321042/sitescripts/notifications/parser.py File sitescripts/notifications/parser.py (right): https://codereview.adblockplus.org/29321041/diff/29321042/sitescripts/notifications/parser.py#newcode93 sitescripts/notifications/parser.py:93: current[key] = dateutil.parser.parse(value) On 2015/06/23 15:51:43, Felix Dahlke wrote: ...
4 years, 2 months ago (2015-06-23 15:58:38 UTC) #5
Wladimir Palant
https://codereview.adblockplus.org/29321041/diff/29321042/sitescripts/notifications/parser.py File sitescripts/notifications/parser.py (right): https://codereview.adblockplus.org/29321041/diff/29321042/sitescripts/notifications/parser.py#newcode93 sitescripts/notifications/parser.py:93: current[key] = dateutil.parser.parse(value) On 2015/06/23 15:58:38, Sebastian Noack wrote: ...
4 years, 2 months ago (2015-06-23 17:24:00 UTC) #6
Felix Dahlke
New patch set is up. https://codereview.adblockplus.org/29321041/diff/29321042/sitescripts/notifications/parser.py File sitescripts/notifications/parser.py (right): https://codereview.adblockplus.org/29321041/diff/29321042/sitescripts/notifications/parser.py#newcode93 sitescripts/notifications/parser.py:93: current[key] = dateutil.parser.parse(value) On ...
4 years, 2 months ago (2015-06-24 07:53:16 UTC) #7
Sebastian Noack
4 years, 2 months ago (2015-06-24 08:11:15 UTC) #8
LGTM
Sign in to reply to this message.

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