Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🎨 Refactor the logging module monolith #3319

Merged

Conversation

ff137
Copy link
Contributor

@ff137 ff137 commented Oct 29, 2024

The existing acapy_agent/config/logging.py module is a monolith, containing multiple logging-related functionalities.

This PR breaks down the module into smaller, more focused components under config/logging/ to improve maintainability and testability. No functional changes are introduced; this is purely a structural reorganization. Everything in the previous logging.py module has been copied directly and split up into bespoke modules.

(Note: This is a pre-cursor PR to introducing loguru as an optional log provider. So, this is some initial refactoring to help streamline that contribution.) (edit: maybe no need for loguru, but the refactoring helps for general maintainability)

@ff137
Copy link
Contributor Author

ff137 commented Oct 29, 2024

5 "security hotspots" identified by sonarcloud -- they seem to be safe to ignore
test coverage is perhaps worth increasing. not what I had in mind :-) but maybe not too hard to generate some unit tests

@ff137 ff137 force-pushed the modularise-logging-monolith branch from 082f71c to 5c27b13 Compare October 29, 2024 19:18
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
5 Security Hotspots
63.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarCloud

@jamshale
Copy link
Contributor

This is ready? Looking good to me 👍

@ff137
Copy link
Contributor Author

ff137 commented Oct 31, 2024

@jamshale I'm happy with it! Tested it and everything looks the same 👍

@jamshale jamshale merged commit 506089c into openwallet-foundation:main Oct 31, 2024
9 of 10 checks passed
@ff137 ff137 deleted the modularise-logging-monolith branch October 31, 2024 18:52
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.

2 participants