Skip to content

[bug/qa] Fix lint errors being ignored (follow-up to #571)#598

Closed
Varadraj75 wants to merge 4 commits intoopenwisp:masterfrom
Varadraj75:issues/485-fix-lint-errors
Closed

[bug/qa] Fix lint errors being ignored (follow-up to #571)#598
Varadraj75 wants to merge 4 commits intoopenwisp:masterfrom
Varadraj75:issues/485-fix-lint-errors

Conversation

@Varadraj75
Copy link

Checklist

  • I have read the OpenWISP Contributing Guidelines.
  • I have manually tested the changes proposed in this pull request.
  • I have written new test cases for new code and/or updated existing tests for changes to existing code.
  • I have updated the documentation.

Reference to Existing Issue

Closes #485 .

Description of Changes

This PR addresses issue #485 by ensuring QA and lint checks are no longer ignored and by fixing the currently solvable lint failures so that CI errors are real and actionable.

Key changes

  • Ensured run-qa-checks properly fails on lint and QA errors
  • Fixed missing newline issues and formatting problems detected by QA
  • Verified QA behavior locally to match CI expectations
  • Avoided suppressing errors (e.g. || true) so failures are correctly reported

Screenshot

Local run-qa-checks execution showing that QA and documentation lint checks
are properly enforced and errors are no longer ignored (RST files reported
as needing reformatting).
Screenshot 2026-02-01 at 3 23 07 AM

Copilot AI review requested due to automatic review settings January 31, 2026 21:54
@coderabbitai
Copy link

coderabbitai bot commented Jan 31, 2026

Walkthrough

This pull request adds Flake8 linting configuration and updates the QA check workflow. Two configuration files are created/updated: a new .flake8 file and updates to pyproject.toml, both specifying exclude patterns for common directories. The run-qa-checks script is modified to remove explicit skip flags for isort, flake8, black, and checkmigrations, enabling these checks by default. Additionally, minor formatting changes are made: string quotes are standardized in a utility script, and whitespace is adjusted in a template file.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main purpose as fixing ignored lint errors and references the follow-up issue, directly summarizing the core change.
Description check ✅ Passed The description addresses the template sections with checklist completion, linked issue reference (#485), detailed change descriptions, and includes a supporting screenshot showing QA check output.
Linked Issues check ✅ Passed The PR addresses all coding objectives from #485: moved QA checks to run-qa-checks script, fixed lint issues, removed error suppression, and configured proper flake8 exclusions.
Out of Scope Changes check ✅ Passed All changes directly support the PR objectives: flake8/QA configuration additions, quote formatting fixes, and blank line corrections address lint errors per issue #485.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c9ee54c and 1b91ec7.

📒 Files selected for processing (5)
  • .flake8
  • files/generate_django_secret_key.py
  • pyproject.toml
  • run-qa-checks
  • templates/load_initial_data.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
  • GitHub Check: Build ubuntu2404
  • GitHub Check: Build ubuntu2204
  • GitHub Check: Build debian12
  • GitHub Check: Build debian13
  • GitHub Check: Agent
🔇 Additional comments (5)
templates/load_initial_data.py (1)

9-9: No functional impact — looks good.
Extra spacing after the docstring is fine.

files/generate_django_secret_key.py (1)

5-6: LGTM — formatting-only change.
String literal style updates are fine.

.flake8 (1)

1-11: LGTM — sensible exclude list.
The exclusions look appropriate for repo-wide linting.

run-qa-checks (1)

4-4: LGTM — clean and focused QA entrypoint.

pyproject.toml (1)

4-10: Remove this comment; there is no .flake8 file in the repository to mirror from.

The suggestion assumes a .flake8 file exists and needs to be mirrored in pyproject.toml, but repository search confirms no .flake8 file exists. Without a source file to reference, this recommendation has no basis.

If excluding templates/** is desired (separate decision), that should be stated as a standalone suggestion without reference to a non-existent .flake8 file.

Likely an incorrect or invalid review comment.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR aims to ensure QA/lint failures are no longer silently skipped by running the full openwisp-qa-check suite and addressing a few currently-fixable formatting issues so CI results are actionable.

Changes:

  • Updates run-qa-checks to run openwisp-qa-check without skip flags.
  • Adds flake8 configuration (via both pyproject.toml and a new .flake8 file).
  • Applies minor Python formatting fixes (docstring spacing and quote style).

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
templates/load_initial_data.py Adds a blank line after the module docstring to satisfy formatting/lint expectations.
run-qa-checks Stops skipping linters/QA checks by invoking openwisp-qa-check directly.
pyproject.toml Adds [tool.flake8] configuration section.
files/generate_django_secret_key.py Adjusts string quoting style for formatting consistency.
.flake8 Introduces a flake8 config with excludes (including templates/**).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@Varadraj75
Copy link
Author

@nemesifier
All CI checks are now green and QA checks are properly enforced locally and in CI.
The PR should be ready for review. Thanks!

Copy link
Member

@nemesifier nemesifier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR is completely missing the point and doesn't really pick up from where #571 stalled. Please revise the changes in that PR and you'll see that yaml and ansible linting is completely missing.

@Varadraj75 I prefer that you pull all the commits from that PR and add your commtis on top of it. We'll save a lot of time and also give credit to the other contributor which is fair.

@Varadraj75
Copy link
Author

Thanks for the clarification @nemesifier, that makes sense.

I’ll rework this by pulling all commits from PR #571 and then applying my fixes on top of it, so we keep the full lint scope (including yaml and ansible linting) and preserve credit for the original contributor.

I’ll update the PR accordingly once done.

@Varadraj75 Varadraj75 closed this Feb 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[bug/qa] Lint errors are ignored

3 participants