refactor: consolidate linting workflow; move more checks to ruff#38468
Draft
refactor: consolidate linting workflow; move more checks to ruff#38468
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Comment on lines
+13
to
+40
| name: Ruff | ||
| runs-on: ubuntu-24.04 | ||
| steps: | ||
| - uses: actions/checkout@v6 | ||
|
|
||
| - name: Setup Python | ||
| uses: actions/setup-python@v6 | ||
| with: | ||
| python-version: "3.12" | ||
|
|
||
| - name: Get pip cache dir | ||
| id: pip-cache-dir | ||
| run: echo "dir=$(pip cache dir)" >> $GITHUB_OUTPUT | ||
|
|
||
| - name: Cache pip dependencies | ||
| uses: actions/cache@v5 | ||
| with: | ||
| path: ${{ steps.pip-cache-dir.outputs.dir }} | ||
| key: ${{ runner.os }}-pip-${{ hashFiles('requirements/edx/testing.txt') }} | ||
| restore-keys: ${{ runner.os }}-pip- | ||
|
|
||
| - name: Install quality requirements | ||
| run: pip install -r requirements/edx/testing.txt | ||
|
|
||
| - name: Run ruff | ||
| run: ruff check --output-format=github . | ||
|
|
||
| policy-checks: |
Comment on lines
41
to
+187
| @@ -73,12 +102,11 @@ jobs: | |||
| run: | | |||
| pip install -e . | |||
|
|
|||
| - name: Run Quality Tests | |||
| - name: Run Policy Checks | |||
| env: | |||
| PIP_SRC: ${{ runner.temp }} | |||
| TARGET_BRANCH: ${{ github.base_ref }} | |||
| run: | | |||
| ruff check --output-format=github . | |||
| make xsslint | |||
| make pii_check | |||
| make check_keywords | |||
| @@ -93,3 +121,82 @@ jobs: | |||
| test_root/log/**/*.log | |||
| *.log | |||
| overwrite: true | |||
|
|
|||
| run-pylint: | |||
| needs: [ruff] | |||
| runs-on: ubuntu-24.04 | |||
| strategy: | |||
| fail-fast: false | |||
| matrix: | |||
| include: | |||
| - module-name: lms-1 | |||
| path: "lms/djangoapps/badges/ lms/djangoapps/branding/ lms/djangoapps/bulk_email/ lms/djangoapps/bulk_enroll/ lms/djangoapps/bulk_user_retirement/ lms/djangoapps/ccx/ lms/djangoapps/certificates/ lms/djangoapps/commerce/ lms/djangoapps/course_api/ lms/djangoapps/course_blocks/ lms/djangoapps/course_home_api/ lms/djangoapps/course_wiki/ lms/djangoapps/coursewarehistoryextended/ lms/djangoapps/debug/ lms/djangoapps/courseware/ lms/djangoapps/course_goals/ lms/djangoapps/rss_proxy/" | |||
| - module-name: lms-2 | |||
| path: "lms/djangoapps/gating/ lms/djangoapps/grades/ lms/djangoapps/instructor/ lms/djangoapps/instructor_analytics/ lms/djangoapps/discussion/ lms/djangoapps/edxnotes/ lms/djangoapps/experiments/ lms/djangoapps/instructor_task/ lms/djangoapps/learner_dashboard/ lms/djangoapps/learner_home/ lms/djangoapps/lms_initialization/ lms/djangoapps/lms_xblock/ lms/djangoapps/lti_provider/ lms/djangoapps/mailing/ lms/djangoapps/mobile_api/ lms/djangoapps/monitoring/ lms/djangoapps/ora_staff_grader/ lms/djangoapps/program_enrollments/ lms/djangoapps/rss_proxy lms/djangoapps/static_template_view/ lms/djangoapps/staticbook/ lms/djangoapps/support/ lms/djangoapps/survey/ lms/djangoapps/teams/ lms/djangoapps/tests/ lms/djangoapps/user_tours/ lms/djangoapps/verify_student/ lms/djangoapps/mfe_config_api/ lms/envs/ lms/lib/ lms/tests.py" | |||
| - module-name: openedx-1 | |||
| path: "openedx/core/types/ openedx/core/djangoapps/ace_common/ openedx/core/djangoapps/agreements/ openedx/core/djangoapps/api_admin/ openedx/core/djangoapps/auth_exchange/ openedx/core/djangoapps/bookmarks/ openedx/core/djangoapps/cache_toolbox/ openedx/core/djangoapps/catalog/ openedx/core/djangoapps/ccxcon/ openedx/core/djangoapps/commerce/ openedx/core/djangoapps/common_initialization/ openedx/core/djangoapps/common_views/ openedx/core/djangoapps/config_model_utils/ openedx/core/djangoapps/content/ openedx/core/djangoapps/content_libraries/ openedx/core/djangoapps/content_staging/ openedx/core/djangoapps/contentserver/ openedx/core/djangoapps/cookie_metadata/ openedx/core/djangoapps/cors_csrf/ openedx/core/djangoapps/course_apps/ openedx/core/djangoapps/course_date_signals/ openedx/core/djangoapps/course_groups/ openedx/core/djangoapps/courseware_api/ openedx/core/djangoapps/crawlers/ openedx/core/djangoapps/credentials/ openedx/core/djangoapps/credit/ openedx/core/djangoapps/dark_lang/ openedx/core/djangoapps/debug/ openedx/core/djangoapps/discussions/ openedx/core/djangoapps/django_comment_common/ openedx/core/djangoapps/embargo/ openedx/core/djangoapps/enrollments/ openedx/core/djangoapps/external_user_ids/ openedx/core/djangoapps/zendesk_proxy/ openedx/core/djangolib/ openedx/core/lib/ openedx/core/djangoapps/course_live/" | |||
| - module-name: openedx-2 | |||
| path: "openedx/core/djangoapps/geoinfo/ openedx/core/djangoapps/header_control/ openedx/core/djangoapps/heartbeat/ openedx/core/djangoapps/lang_pref/ openedx/core/djangoapps/models/ openedx/core/djangoapps/monkey_patch/ openedx/core/djangoapps/oauth_dispatch/ openedx/core/djangoapps/olx_rest_api/ openedx/core/djangoapps/password_policy/ openedx/core/djangoapps/plugin_api/ openedx/core/djangoapps/plugins/ openedx/core/djangoapps/profile_images/ openedx/core/djangoapps/programs/ openedx/core/djangoapps/safe_sessions/ openedx/core/djangoapps/schedules/ openedx/core/djangoapps/service_status/ openedx/core/djangoapps/session_inactivity_timeout/ openedx/core/djangoapps/signals/ openedx/core/djangoapps/site_configuration/ openedx/core/djangoapps/system_wide_roles/ openedx/core/djangoapps/theming/ openedx/core/djangoapps/user_api/ openedx/core/djangoapps/user_authn/ openedx/core/djangoapps/util/ openedx/core/djangoapps/verified_track_content/ openedx/core/djangoapps/video_config/ openedx/core/djangoapps/video_pipeline/ openedx/core/djangoapps/waffle_utils/ openedx/core/djangoapps/xblock/ openedx/core/djangoapps/xmodule_django/ openedx/core/tests/ openedx/features/ openedx/testing/ openedx/tests/ openedx/envs/ openedx/core/djangoapps/notifications/ openedx/core/djangoapps/staticfiles/ openedx/core/djangoapps/content_tagging/ openedx/core/djangoapps/authz/" | |||
| - module-name: common | |||
| path: "common" | |||
| - module-name: cms | |||
| path: "cms" | |||
| - module-name: xmodule | |||
| path: "xmodule" | |||
|
|
|||
| name: pylint ${{ matrix.module-name }} | |||
| steps: | |||
| - name: Check out repo | |||
| uses: actions/checkout@v6 | |||
|
|
|||
| - name: Install required system packages | |||
| run: sudo apt-get update && sudo apt-get install libxmlsec1-dev | |||
|
|
|||
| - name: Set up Python | |||
| uses: actions/setup-python@v6 | |||
| with: | |||
| python-version: "3.12" | |||
|
|
|||
| - name: Get pip cache dir | |||
| id: pip-cache-dir | |||
| run: | | |||
| echo "dir=$(pip cache dir)" >> $GITHUB_OUTPUT | |||
|
|
|||
| - name: Cache pip dependencies | |||
| id: cache-dependencies | |||
| uses: actions/cache@v5 | |||
| with: | |||
| path: ${{ steps.pip-cache-dir.outputs.dir }} | |||
| key: ${{ runner.os }}-pip-${{ hashFiles('requirements/edx/development.txt') }} | |||
| restore-keys: ${{ runner.os }}-pip- | |||
|
|
|||
| - name: Install required Python dependencies | |||
| run: | | |||
| # dev-requirements is needed because the linter will otherwise | |||
| # trip over some dev-only things like django-debug-toolbar | |||
| # (import debug_toolbar) that aren't in testing.txt. | |||
| make dev-requirements | |||
| # After all requirements are installed, check that they're consistent with each other | |||
| pip check | |||
|
|
|||
| - name: Run quality tests | |||
| run: | | |||
| pylint ${{ matrix.path }} | |||
|
|
|||
| # This job aggregates test results. It's the required check for branch protection. | |||
| # https://github.com/marketplace/actions/alls-green#why | |||
feanil
force-pushed
the
feanil/ruff_before_pylint
branch
from
bd5b6f5 to
f38d119
Compare
Merges pylint-checks.yml into quality-checks.yml so the two jobs run in a single workflow with an explicit dependency chain: ruff runs first, then policy-checks (xsslint/pii/keywords) and the pylint matrix run in parallel gated on ruff passing. The alls-green aggregation job is renamed from "Pylint checks successful" to "Quality checks successful" to reflect its broader scope. Branch protection required checks to update: "Pylint checks successful" → "Quality checks successful" "Quality Others" → "Policy & Security Checks" (if currently required) Push trigger broadened from open-release/lilac.master to release/** to cover all release branches generically. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…d by ruff) Both are already caught by ruff F401 and F821 respectively. Running them in pylint as well is redundant and wastes CI time. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Removes bare-except from pylint's disabled list and drops E722 from ruff's ignore list so ruff now enforces it. Existing violations are suppressed in a follow-up mechanical commit. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Mechanical: ruff --select E722 --add-noqa applied to suppress existing bare-except violations. Inline # pylint: disable=bare-except comments removed from 42 sites since pylint no longer checks this rule. lint-amnesty annotations removed from touched lines. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Removes line-too-long from pylint's disabled list and drops E501 from ruff's ignore list so ruff now enforces it. Existing violations are suppressed in a follow-up mechanical commit. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
feanil
force-pushed
the
feanil/ruff_before_pylint
branch
from
f38d119 to
c102e6d
Compare
|
|
||
| # No enrollments | ||
| expected = hashlib.md5(self.user.username.encode('utf-8')).hexdigest() # lint-amnesty, pylint: disable=no-member | ||
| expected = hashlib.md5(self.user.username.encode('utf-8')).hexdigest() # pylint: disable=no-member |
Mechanical: ruff --select E501 --add-noqa applied to suppress existing violations. All pylint disable=line-too-long comments removed. lint-amnesty annotations removed from touched lines. Suppressions that became redundant after lint-amnesty removal cleaned up with ruff --extend-select RUF100 --fix. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… E741 Fixes 9 occurrences of the ambiguous single-character name `l` (easily mistaken for the digit 1) across 6 files and removes E741 from ruff's ignore list so future code is checked automatically. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
RUF100 flags # noqa comments that no longer suppress any active violation, keeping the suppression list accurate as code evolves. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
feanil
force-pushed
the
feanil/ruff_before_pylint
branch
from
c102e6d to
fb29154
Compare
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
2 participants
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.
Summary
pylint-checks.ymlintoquality-checks.ymlso all linting runs in a single workflow with an explicit dependency chain: ruff runs first, thenpolicy-checks(xsslint/pii/keywords) and the pylint matrix run in parallel, both gated on ruff passing. Pylint matrix jobs are skipped entirely if ruff fails, saving CI minutes.bare-except(E722),line-too-long(E501),unused-import(F401), andundefined-variable(F821) from pylint to ruff, removing the redundant coverage from the slow linter.lnames and re-enables ruff E741 enforcement going forward.open-release/lilac.mastertorelease/**.Before merging — update branch protection required checks
Pylint checks successfulQuality checks successfulQuality Others(if currently required)Policy & Security Checks