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

#19205: Don't import re and sysconfig in site.py

Can't Edit
Can't Publish+Mail
Start Review
Created:
6 years ago by lists
Modified:
6 years ago
Reviewers:
pitrou
CC:
barry, AntoinePitrou, haypo, christian.heimes, devnull_psf.upfronthosting.co.za
Visibility:
Public.

Patch Set 1 #

Patch Set 2 #

Patch Set 3 #

Total comments: 5
Unified diffs Side-by-side diffs Delta from patch set Stats Patch
Lib/locale.py View 1 2 4 chunks +13 lines, -5 lines 2 comments Download
Lib/site.py View 1 2 3 chunks +6 lines, -4 lines 1 comment Download
Lib/sysconfig.py View 1 2 4 chunks +3 lines, -1 line 0 comments Download
Lib/test/test_site.py View 1 2 2 chunks +15 lines, -0 lines 2 comments Download

Messages

Total messages: 1
AntoinePitrou
6 years ago #1
http://bugs.python.org/review/19205/diff/9505/Lib/locale.py
File Lib/locale.py (right):

http://bugs.python.org/review/19205/diff/9505/Lib/locale.py#newcode193
Lib/locale.py:193: _percent_re = re.compile(_percent_re)
Please don't do that. Just call re.match(_percent_re, ...).

http://bugs.python.org/review/19205/diff/9505/Lib/locale.py#newcode230
Lib/locale.py:230: if isinstance(_percent_re, str):
Same.

http://bugs.python.org/review/19205/diff/9505/Lib/site.py
File Lib/site.py (right):

http://bugs.python.org/review/19205/diff/9505/Lib/site.py#newcode463
Lib/site.py:463: CONFIG_LINE = re.compile(r'^(?P<key>(\w|[-_])+)\s*'
Again, please don't.

http://bugs.python.org/review/19205/diff/9505/Lib/test/test_site.py
File Lib/test/test_site.py (right):

http://bugs.python.org/review/19205/diff/9505/Lib/test/test_site.py#newcode432
Lib/test/test_site.py:432: modules = set(ast.literal_eval(stdout))
Why not eval()? Let's not be pedantic.

http://bugs.python.org/review/19205/diff/9505/Lib/test/test_site.py#newcode435
Lib/test/test_site.py:435: self.assertFalse(modules.intersection(re_mods))
`modules & re_mods` looks clearer to me.
Sign in to reply to this message.

RSS Feeds Recent Issues | This issue
This is Rietveld 894c83f36cb7+