Skip to content

Conversation

@hatimoua
Copy link
Contributor

@hatimoua hatimoua commented Dec 5, 2025

Description:
This PR addresses Issue #1176 by auditing and improving docstrings in the pyrit/memory module. It ensures full compliance with Google Style guidelines and Ruff checks.

Changes:

  • Removed pyrit/memory from tool.ruff.lint.per-file-ignores in pyproject.toml.
  • Added missing docstrings for __init__, magic methods, and public interfaces across all memory backends (Azure SQL, SQLite, MemoryInterface).
  • Added explicit Args, Returns, and Raises sections to document parameters, return types, and exceptions (ValueError, SQLAlchemyError).
  • Fixed imperative mood violations (D401) and missing argument descriptions (D417).

Verification:

  • Ran ruff check pyrit/memory locally.
  • Result: All checks passed!

Linked Issue:
Addresses a part of #1176

@hatimoua
Copy link
Contributor Author

hatimoua commented Dec 5, 2025 via email

@hatimoua
Copy link
Contributor Author

hatimoua commented Dec 5, 2025 via email

@rlundeen2
Copy link
Contributor

Looks good, but can you update pre-commits, they are failing.

You can run these locally with pre-commit run --all-files. You can see the existing errors here: https://github.com/Azure/PyRIT/actions/runs/19995549971/job/57342515969?pr=1223

@rlundeen2 rlundeen2 self-assigned this Dec 7, 2025
@hatimoua hatimoua force-pushed the docstrings/memory-1176 branch from 05e38b2 to 0343ddd Compare December 7, 2025 13:29
@hatimoua
Copy link
Contributor Author

hatimoua commented Dec 7, 2025

@rlundeen2 Thanks for the review!

I have run pre-commit run --all-files and fixed the formatting issues.
I also squashed the commits to clean up a Git identity configuration error on my end.

Ready for re-review!

@romanlutz
Copy link
Contributor

@rlundeen2 Thanks for the review!

I have run pre-commit run --all-files and fixed the formatting issues. I also squashed the commits to clean up a Git identity configuration error on my end.

Ready for re-review!

@hatimoua there's a comment in this thread by microsoft-github-policy-service. For us to be able to accept the PR, you'll have to agree to the terms there.

@hatimoua
Copy link
Contributor Author

hatimoua commented Dec 7, 2025

@microsoft-github-policy-service agree

@romanlutz
Copy link
Contributor

There are conflicts with the base branch. You probably need to merge in latest main. There have been changes to the toml file 🙂

@hatimoua
Copy link
Contributor Author

hatimoua commented Dec 7, 2025

@romanlutz I've merged the latest main branch and updated pyproject.toml to include the recent upstream changes while keeping the memory module enabled for linting.

@romanlutz
Copy link
Contributor

@hatimoua something went wrong with the merge. There are files added now. Please take a moment to remove those.

@romanlutz romanlutz merged commit a1a709b into Azure:main Dec 9, 2025
20 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