Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1226)

Issue 739073004: pylintrc: Disable R0401: Cyclic Import (Closed)

Created:
6 years, 1 month ago by hinoka
Modified:
5 years, 2 months ago
Reviewers:
dnj, Ryan Tseng
CC:
chromium-reviews, Dirk Pranke, cmp-cc_chromium.org, iannucci+depot_tools_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/tools/depot_tools.git@master
Project:
tools
Visibility:
Public.

Description

pylintrc: Disable R0401: Cyclic Import Theres no way to disable this on a per-file basis, it can only be done globally. Therefore, disabling this warning until the infra/ problem can be looked at and fixed. BUG=

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+2 lines, -1 line) Patch
M pylintrc View 2 chunks +2 lines, -1 line 0 comments Download

Messages

Total messages: 5 (1 generated)
Ryan Tseng
6 years, 1 month ago (2014-11-19 20:31:05 UTC) #2
dnj
It seems like cyclic import warnings might be useful.
6 years, 1 month ago (2014-11-19 20:47:02 UTC) #3
Ryan Tseng
You're right, but short of figuring out how to fix the one broken case in ...
6 years, 1 month ago (2014-11-19 20:50:45 UTC) #4
dnj
6 years, 1 month ago (2014-11-19 21:26:33 UTC) #5
On 2014/11/19 20:50:45, Ryan Tseng wrote:
> You're right, but short of figuring out how to fix the one broken case in
infra
> (which isn't even a real cyclic dependency.  Its the same package importing
> something from the same project, so what the heck pylint), this is the other
way
> I can think of to land my otherwise unrelated and trivial change to infra
today.
> 
> And I'd argue that since we didn't have it before everything broke
(yesterday),
> its  not really a regression.

Agree on not a regression. Can always dcommit until this is figured out :(

Maybe try running the pylint line with '--import-graph=asdf.dot'?

I'm just worried that disabling this globally might effectively mean it will not
be re-enabled.

Powered by Google App Engine
This is Rietveld 408576698