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

[Issue #1541] Fail CI when there are outstanding linter changes #1583

Merged
merged 1 commit into from
Apr 1, 2024

Conversation

coilysiren
Copy link
Collaborator

@coilysiren coilysiren commented Apr 1, 2024

Summary

Fixes #1541

Time to review: 2 mins

Changes proposed

Splits lint into:

  • format
  • format-check
  • lint

Their purposes are explained inline

Context for reviewers

See #1541

@@ -184,15 +184,9 @@ def _isolate_data_for_this_sprint(self) -> pd.DataFrame:

# """
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ruff made these changes. This is a good reminder not to leave large code blocks commented out like this, its "unsafe"

@@ -10,7 +10,9 @@


def get_daily_tix_counts_by_status(
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

black made these changes

@coilysiren coilysiren changed the title workon linters [Issue #1541] Fail CI when there are outstanding linter changes Apr 1, 2024
@coilysiren coilysiren marked this pull request as ready for review April 1, 2024 17:12
@coilysiren coilysiren requested a review from widal001 April 1, 2024 17:12
Copy link
Collaborator

@chouinar chouinar left a comment

Choose a reason for hiding this comment

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

LGTM - follows the same pattern as the API.

@coilysiren coilysiren merged commit 2251ce4 into main Apr 1, 2024
8 checks passed
@coilysiren coilysiren deleted the analytics-linters-2 branch April 1, 2024 17:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Task]: Delivery Dashboard - Fail CI when there are outstanding linter changes
2 participants