Skip to content

Conversation

agseco
Copy link
Contributor

@agseco agseco commented Mar 7, 2025

Motivation

First time or sporadic contributors may have missed installing the pre-commit hook. In this situation, un-formatted code may be accidentally pushed. In such case, the output of the linter is not very informative:

Would reformat: tests/aws/services/kms/test_kms.py
1 file would be reformatted, 1815 files already formatted
make: *** [Makefile:121: lint] Error 1

Exited with code exit status 2

The goal is to make the output of the linter more informative, so the actual cause of the problem is clearer which could result in a better developer experience.

Changes

Adding the --diff option to the python -m ruff format --check . command.

So the output would show the difference between the current file and how the formatted file would look like. For instance, the previous example would look like this:

python -m ruff format --check --diff .
--- tests/aws/services/kms/test_kms.py
+++ tests/aws/services/kms/test_kms.py
@@ -1099,7 +1099,9 @@
         assert aws_client.kms.get_key_rotation_status(KeyId=key_id)["KeyRotationEnabled"] is False
 
     @markers.aws.validated
-    def test_rotate_key_on_demand_succeeds_given_symmetric_key_preserving_automatic_rotation_schedules(self, kms_key, aws_client):
+    def test_rotate_key_on_demand_succeeds_given_symmetric_key_preserving_automatic_rotation_schedules(
+        self, kms_key, aws_client
+    ):
         key_id = kms_key["KeyId"]
 
         rotation_status_response_before = aws_client.kms.get_key_rotation_status(KeyId=key_id)

If the output is too verbose, | top 10 could be added to trim the output while still being informative.

Unfortunately ruff doesn't seem to have a less verbose output as other linters may have, --diff seems the only option available to get feedback of the problems found.

@alexrashed alexrashed self-requested a review March 7, 2025 09:39
@alexrashed alexrashed added the semver: patch Non-breaking changes which can be included in patch releases label Mar 7, 2025
@alexrashed alexrashed added this to the 4.3 milestone Mar 7, 2025
Copy link
Member

@alexrashed alexrashed left a comment

Choose a reason for hiding this comment

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

Lookin' good! Thanks for proactively improving the internal dev experience for all LocalStack Contributors! 🚀 💯

@alexrashed alexrashed merged commit 3fa9a9e into localstack:master Mar 10, 2025
37 of 38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver: patch Non-breaking changes which can be included in patch releases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants