Patchwork [1,of,4] import-checker: allow symbol imports from hgweb.common and .request

login
register
mail settings
Submitter Yuya Nishihara
Date Nov. 18, 2015, 3:44 p.m.
Message ID <d66415991a1f94da2f7f.1447861463@mimosa>
Download mbox | patch
Permalink /patch/11481/
State Superseded
Commit e5be48dd8215c63e6b79c2926d707e9b3ff2c504
Delegated to: Pierre-Yves David
Headers show

Comments

Yuya Nishihara - Nov. 18, 2015, 3:44 p.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1446353721 -32400
#      Sun Nov 01 13:55:21 2015 +0900
# Node ID d66415991a1f94da2f7f1e24793746a48deb3df6
# Parent  a21e63ed613bc956c88c2832fce1968550ccf834
import-checker: allow symbol imports from hgweb.common and .request

This seems the convention of hgweb.
Pierre-Yves David - Nov. 18, 2015, 9:51 p.m.
On 11/18/2015 07:44 AM, Yuya Nishihara wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1446353721 -32400
> #      Sun Nov 01 13:55:21 2015 +0900
> # Node ID d66415991a1f94da2f7f1e24793746a48deb3df6
> # Parent  a21e63ed613bc956c88c2832fce1968550ccf834
> import-checker: allow symbol imports from hgweb.common and .request
>
> This seems the convention of hgweb.

I'm unclear about what this changeset achieve and why it is necessary. 
Can you elaborate?

--
Pierre-Yves David
Gregory Szorc - Nov. 19, 2015, 2:42 a.m.
On Wed, Nov 18, 2015 at 1:51 PM, Pierre-Yves David <
pierre-yves.david@ens-lyon.org> wrote:

>
>
> On 11/18/2015 07:44 AM, Yuya Nishihara wrote:
>
>> # HG changeset patch
>> # User Yuya Nishihara <yuya@tcha.org>
>> # Date 1446353721 -32400
>> #      Sun Nov 01 13:55:21 2015 +0900
>> # Node ID d66415991a1f94da2f7f1e24793746a48deb3df6
>> # Parent  a21e63ed613bc956c88c2832fce1968550ccf834
>> import-checker: allow symbol imports from hgweb.common and .request
>>
>> This seems the convention of hgweb.
>>
>
> I'm unclear about what this changeset achieve and why it is necessary. Can
> you elaborate?
>
>
absolute_import engages a strict mode about what imports are allowed. One
of the rules is we only allow importing of symbols that are themselves
modules (as opposed to functions, variables, etc). We're supposed to import
modules then assign their symbols to locals post import.

The list modified by this patch stops applying this restriction to hgweb
because it is easier to do that than rewrite half of hgweb to not import
symbols directly.
Pierre-Yves David - Nov. 19, 2015, 5:51 a.m.
On 11/18/2015 06:42 PM, Gregory Szorc wrote:
> On Wed, Nov 18, 2015 at 1:51 PM, Pierre-Yves David
> <pierre-yves.david@ens-lyon.org <mailto:pierre-yves.david@ens-lyon.org>>
> wrote:
>
>
>
>     On 11/18/2015 07:44 AM, Yuya Nishihara wrote:
>
>         # HG changeset patch
>         # User Yuya Nishihara <yuya@tcha.org <mailto:yuya@tcha.org>>
>         # Date 1446353721 -32400
>         #      Sun Nov 01 13:55:21 2015 +0900
>         # Node ID d66415991a1f94da2f7f1e24793746a48deb3df6
>         # Parent  a21e63ed613bc956c88c2832fce1968550ccf834
>         import-checker: allow symbol imports from hgweb.common and .request
>
>         This seems the convention of hgweb.
>
>
>     I'm unclear about what this changeset achieve and why it is
>     necessary. Can you elaborate?
>
>
> absolute_import engages a strict mode about what imports are allowed.
> One of the rules is we only allow importing of symbols that are
> themselves modules (as opposed to functions, variables, etc). We're
> supposed to import modules then assign their symbols to locals post import.
>
> The list modified by this patch stops applying this restriction to hgweb
> because it is easier to do that than rewrite half of hgweb to not import
> symbols directly.

Okay pushed to the clowncopter.

Patch

diff --git a/contrib/import-checker.py b/contrib/import-checker.py
--- a/contrib/import-checker.py
+++ b/contrib/import-checker.py
@@ -14,6 +14,8 @@  import zlib
 # Whitelist of modules that symbols can be directly imported from.
 allowsymbolimports = (
     '__future__',
+    'mercurial.hgweb.common',
+    'mercurial.hgweb.request',
     'mercurial.i18n',
     'mercurial.node',
 )