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

chore: update ruff linting scripts and settings #1105

Merged
merged 10 commits into from
Sep 26, 2024

Conversation

anakin87
Copy link
Member

@anakin87 anakin87 commented Sep 25, 2024

Proposed Changes:

  • I initially noticed that the script hatch run lint:fmt was not working -> I replaced ruff --fix with ruff check --fix.

  • Then I started noticing some warnings like warning: The top-level linter settings are deprecated in favour of their counterparts in the lint section... -> I changed all of them in pyproject files.

  • I also noticed that we introduced scripts like this: style = ["ruff check {args:. --exclude tests/}",...] in recent PRs.

    • I think it is preferable to specify exclusions in structured form in pyproject file (using tool.ruff.exclude = ["tests"])
    • I would not exclude tests from linting and formatting.

    -> I removed these expressions and, only where necessary (too much work to adopt linting), replaced them with the appropriate settings in the pyproject file.

How did you test it?

CI, local tests.

Checklist

@anakin87 anakin87 changed the title Update ruff commands settings chore: update ruff liniting commands and settings Sep 25, 2024
@anakin87 anakin87 changed the title chore: update ruff liniting commands and settings chore: update ruff linting scripts and settings Sep 25, 2024
@@ -15,7 +15,7 @@ defaults:
working-directory: integrations/anthropic

concurrency:
group: cohere-${{ github.head_ref }}
group: anthropic-${{ github.head_ref }}
Copy link
Member Author

Choose a reason for hiding this comment

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

unrelated: I noticed that Anthropic tests were not running due to the wrong concurrency group.

@anakin87 anakin87 marked this pull request as ready for review September 26, 2024 07:13
@anakin87 anakin87 requested a review from a team as a code owner September 26, 2024 07:13
@anakin87 anakin87 requested review from Amnah199 and removed request for a team September 26, 2024 07:13
@@ -7,6 +7,7 @@
from haystack.components.generators.utils import print_streaming_chunk
from haystack.dataclasses import ChatMessage, ChatRole, StreamingChunk
from haystack.utils import Secret

Copy link
Contributor

Choose a reason for hiding this comment

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

Was this intentional? It added extra files in PR.

Copy link
Member Author

@anakin87 anakin87 Sep 26, 2024

Choose a reason for hiding this comment

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

yeah, related to formatting...

Copy link
Contributor

@silvanocerza silvanocerza left a comment

Choose a reason for hiding this comment

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

Niiiice! 🚀

@anakin87 anakin87 merged commit 907c10b into main Sep 26, 2024
159 checks passed
@anakin87 anakin87 deleted the update-ruff-commands-settings branch September 26, 2024 15:50
Amnah199 pushed a commit that referenced this pull request Oct 2, 2024
* updates

* more changes

* lint

* more changes

* more changes

* mmore and more changes

* right concurrency group for anthropic

* apply suggestions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment