john-bodley commented on code in PR #24700:
URL: https://github.com/apache/superset/pull/24700#discussion_r1270055291


##########
.pylintrc:
##########
@@ -86,6 +86,7 @@ disable=
     missing-docstring,
     duplicate-code,
     unspecified-encoding,
+    cyclic-import,

Review Comment:
   @EugeneTorap as far as I'm aware the `cyclic-import` error existed in Pylint 
2.9.6 and thus I'm somewhat perplexed as why this is significantly more 
problematic now. In general we shouldn't be relaxing most Pylint rules and thus 
I'm somewhat hesitant to approve something which in theory could lead to a 
regression.
   
   When I pulled your branch, removed line #89 from the `.pylintrc` file, and 
ran, 
   
   ```
   python3.9 -m pip install -r requirements/integration.txt
   tox -e pylint
   ```
   
   I didn't see any `cyclic-import` errors being reported.
   
   Note I did see some errors with the `.pylintrc` file:
   
   ```
   .pylintrc:1:0: E0015: Unrecognized option found: optimize-ast, files-output, 
argument-name-hint, method-name-hint, variable-name-hint, inlinevar-name-hint, 
const-name-hint, class-name-hint, class-attribute-name-hint, module-name-hint, 
attr-name-hint, function-name-hint, no-space-check (unrecognized-option)
   ```
   
   it seems like some of these options are likely deprecated.
   
    



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to