Skip to content

Conversation

ff137
Copy link
Contributor

@ff137 ff137 commented Oct 7, 2025

Material changes in this PR:

  • 🎨 Stricter docstring enforcement:
    • Remove blank lines after docstrings
    • Require a blank line at the end of multiline docstring
  • 🔧 Add isort to selected ruff rules (enforces import sorting)

The motivation for the stricter docstring enforcement is coming from the fact that the large Kanon contribution (#3850) includes many micro-format changes (adding/removing blank lines) that touches many files. These types of changes should be applied automatically and enforced by lint checkers, so we don't have to worry about things like that.

If merged before the Kanon PR, this will make reviewing the actual diff much easier.


Immaterial changes in this PR:

  • ♻️ Sync ruff version in workflows (no format changes between 0.12 -> 0.13)
  • 🔧 Remove unnecessary lint.ignore rules (previously ignored rules that have at some point been fixed / make no changes to code)
  • 🎨 Add comments describing each lint rule being ignored and selected
  • 🎨 Add a recommended lint rules to be added later (commented out)
    • e.g.: ANN (type annotation checks), ASYNC (detect blocking methods being called in async code), B (flake8-bugbear, detect likely bugs), etc.

ff137 added 11 commits October 7, 2025 10:30
These rules make no difference to codebase when lint check is applied, and therefore aren't needed

Signed-off-by: ff137 <[email protected]>
"C" is not a valid ruff rule; it is selecting "C90", which only has 1 rule, C901, which is explicitly being ignored. Therefore, emphasise that this should be fixed later.

Signed-off-by: ff137 <[email protected]>
Copy link

sonarqubecloud bot commented Oct 7, 2025

Quality Gate Failed Quality Gate failed

Failed conditions
3.4% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

@ff137 ff137 requested a review from a team October 7, 2025 09:54
@ff137
Copy link
Contributor Author

ff137 commented Oct 7, 2025

For reviewers: I recommend going through each commit: https://github.com/openwallet-foundation/acapy/pull/3900/commits
I kept commits granular to help with reviewing the large number of files changed

@ff137 ff137 mentioned this pull request Oct 7, 2025
Copy link
Contributor

@jamshale jamshale left a comment

Choose a reason for hiding this comment

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

I'm fine with this 👍

Copy link
Contributor

@swcurran swcurran left a comment

Choose a reason for hiding this comment

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

LGTM

@swcurran swcurran merged commit cfacb07 into openwallet-foundation:main Oct 7, 2025
10 of 11 checks passed
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.

3 participants