Closed
Conversation
bf4ccc9 to
ec3d3b8
Compare
d4341cb to
f470f96
Compare
2d0cb2f to
847e816
Compare
f9068d7 to
a366784
Compare
119216c to
d42b0c4
Compare
2b74022 to
1e7179a
Compare
1e7179a to
10e0539
Compare
3 tasks
c893bf8 to
cfc4415
Compare
10e0539 to
b70a45f
Compare
699ed5e to
a3b6b9a
Compare
5d34f5e to
1a3bcc6
Compare
5d23d41 to
01f8e22
Compare
01f8e22 to
eaeebcf
Compare
kdmccormick
commented
Jun 2, 2022
kdmccormick
commented
Jun 2, 2022
kdmccormick
commented
Jun 2, 2022
Member
Author
|
Ugh, the pylint build just started failing with something wacky. Once #30533 is through, I'll rebase, which'll hopefully resolve pylint. |
eaeebcf to
b7efeb8
Compare
05a65b0 to
e0fcac5
Compare
Member
Author
|
Huh, pylint's failing with something weird. I'll look into that soon. |
Member
Author
|
Closed in favor of #31062 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Blockers
__init__.pyDescription
This PR installs and configures
import-linter. We start with a fairly light set of constraints; for example, we declare thatlms_should_not_depend_on_cms, with a few existing exceptions. Over time, we could make these constraints stricter. The linter is run as a GitHub Action on PR builds and the master branch.Rationale
In order to empower refactoring, restructuring, and extraction of edx-platform code, we would like to be able to analyze and make assertions about what code depends on what. One way to do this is by computing the digraph of Python import statements in our codebase, and failing PR builds if unwanted imports exist in the graph. This is exactly what the tool import-linter will allow us to do.
Example
LMS-specific code belongs in ./lms, and CMS-specific code belongs in ./cms. Since we want to decouple LMS and CMS, we don’t want code in one of these folders to import code in the other.
Using import-linter , we can fail any PR that adds a cms import to a module within lms. We can add exceptions for existing imports.
Supporting information
See BOM-2576 for context and details.
Testing instructions
Run
make lint-importslocally withinlms-shellandstudio-shell. Make sure the check passes.Add an illegal import (for example,
from cms.djangoapps.contentstore import viewswithin anylmsmodule). Make sure the check fails, and make sure the output is comprehensible.Deadline
None.
Other information
import-linterfailures that they see on their PRs. We'll want to educate folks on the importance of these checks so that they don't get frustrated and/or just add their forbidden imports to the "ignore" list.