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

Issue 29319007: Issue 2711 - Refactored ChainedConfigParser, allowing manipulation of list items (Closed)

Created:
June 22, 2015, 9:52 p.m. by Sebastian Noack
Modified:
July 9, 2015, 1:08 p.m.
Reviewers:
Wladimir Palant
Visibility:
Public.

Description

Issue 2711 - Refactored ChainedConfigParser, allowing manipulation of list items

Patch Set 1 : #

Total comments: 24

Patch Set 2 : Addressed comments #

Total comments: 2

Patch Set 3 : Don't rely on internal APIs to work around add_section("default") bug #

Patch Set 4 : Unimplement add_section as well #

Patch Set 5 : Don't convert options to lowercase and decode config files as UTF-8 #

Total comments: 4

Patch Set 6 : Addressed comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+136 lines, -77 lines) Patch
M chainedconfigparser.py View 1 2 3 4 5 3 chunks +132 lines, -76 lines 0 comments Download
M packager.py View 1 2 3 4 5 1 chunk +4 lines, -1 line 0 comments Download

Messages

Total messages: 14
Sebastian Noack
June 22, 2015, 9:53 p.m. (2015-06-22 21:53:44 UTC) #1
Wladimir Palant
https://codereview.adblockplus.org/29319007/diff/29319014/chainedconfigparser.py File chainedconfigparser.py (right): https://codereview.adblockplus.org/29319007/diff/29319014/chainedconfigparser.py#newcode37 chainedconfigparser.py:37: have a source attribute serving the same purpose. Extend ...
June 23, 2015, 9:43 a.m. (2015-06-23 09:43:36 UTC) #2
Sebastian Noack
https://codereview.adblockplus.org/29319007/diff/29319014/chainedconfigparser.py File chainedconfigparser.py (right): https://codereview.adblockplus.org/29319007/diff/29319014/chainedconfigparser.py#newcode37 chainedconfigparser.py:37: have a source attribute serving the same purpose. On ...
June 24, 2015, 8:51 a.m. (2015-06-24 08:51:10 UTC) #3
Wladimir Palant
https://codereview.adblockplus.org/29319007/diff/29319014/chainedconfigparser.py File chainedconfigparser.py (right): https://codereview.adblockplus.org/29319007/diff/29319014/chainedconfigparser.py#newcode93 chainedconfigparser.py:93: self._sections[section] = self._dict() On 2015/06/24 08:51:09, Sebastian Noack wrote: ...
June 25, 2015, 2:11 p.m. (2015-06-25 14:11:40 UTC) #4
Sebastian Noack
https://codereview.adblockplus.org/29319007/diff/29319014/chainedconfigparser.py File chainedconfigparser.py (right): https://codereview.adblockplus.org/29319007/diff/29319014/chainedconfigparser.py#newcode93 chainedconfigparser.py:93: self._sections[section] = self._dict() On 2015/06/25 14:11:40, Wladimir Palant wrote: ...
June 25, 2015, 4:06 p.m. (2015-06-25 16:06:28 UTC) #5
Wladimir Palant
https://codereview.adblockplus.org/29319007/diff/29319014/chainedconfigparser.py File chainedconfigparser.py (right): https://codereview.adblockplus.org/29319007/diff/29319014/chainedconfigparser.py#newcode93 chainedconfigparser.py:93: self._sections[section] = self._dict() On 2015/06/25 16:06:28, Sebastian Noack wrote: ...
June 25, 2015, 4:12 p.m. (2015-06-25 16:12:28 UTC) #6
Sebastian Noack
https://codereview.adblockplus.org/29319007/diff/29319014/chainedconfigparser.py File chainedconfigparser.py (right): https://codereview.adblockplus.org/29319007/diff/29319014/chainedconfigparser.py#newcode93 chainedconfigparser.py:93: self._sections[section] = self._dict() On 2015/06/25 16:12:28, Wladimir Palant wrote: ...
June 25, 2015, 11:05 p.m. (2015-06-25 23:05:31 UTC) #7
Wladimir Palant
https://codereview.adblockplus.org/29319007/diff/29319014/chainedconfigparser.py File chainedconfigparser.py (right): https://codereview.adblockplus.org/29319007/diff/29319014/chainedconfigparser.py#newcode93 chainedconfigparser.py:93: self._sections[section] = self._dict() On 2015/06/25 23:05:31, Sebastian Noack wrote: ...
June 26, 2015, 1:17 p.m. (2015-06-26 13:17:22 UTC) #8
Sebastian Noack
https://codereview.adblockplus.org/29319007/diff/29319014/chainedconfigparser.py File chainedconfigparser.py (right): https://codereview.adblockplus.org/29319007/diff/29319014/chainedconfigparser.py#newcode93 chainedconfigparser.py:93: self._sections[section] = self._dict() So your concerns here are only ...
June 26, 2015, 1:37 p.m. (2015-06-26 13:37:53 UTC) #9
Wladimir Palant
LGTM
June 27, 2015, 6:53 a.m. (2015-06-27 06:53:55 UTC) #10
Sebastian Noack
I found two regressions, fixed with the new patch set: 1. Config files weren't decoded ...
July 7, 2015, 1:09 p.m. (2015-07-07 13:09:50 UTC) #11
Wladimir Palant
https://codereview.adblockplus.org/29319007/diff/29321431/chainedconfigparser.py File chainedconfigparser.py (right): https://codereview.adblockplus.org/29319007/diff/29321431/chainedconfigparser.py#newcode47 chainedconfigparser.py:47: As opposed to ChainedConfigParser, files are decoded as UTF-8 ...
July 7, 2015, 2:53 p.m. (2015-07-07 14:53:18 UTC) #12
Sebastian Noack
https://codereview.adblockplus.org/29319007/diff/29321431/chainedconfigparser.py File chainedconfigparser.py (right): https://codereview.adblockplus.org/29319007/diff/29321431/chainedconfigparser.py#newcode47 chainedconfigparser.py:47: As opposed to ChainedConfigParser, files are decoded as UTF-8 ...
July 7, 2015, 3:20 p.m. (2015-07-07 15:20:44 UTC) #13
Wladimir Palant
July 8, 2015, 10:03 p.m. (2015-07-08 22:03:52 UTC) #14
LGTM

Powered by Google App Engine
This is Rietveld