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

Style ci update #23

Merged
merged 5 commits into from
Apr 27, 2024
Merged

Conversation

Domejko
Copy link
Contributor

@Domejko Domejko commented Apr 27, 2024

  • Changed black command to check code. If it will find any files that need formatting it will fail CI.
  • Added type: ignore on missing import in simple_tests.py

@Domejko
Copy link
Contributor Author

Domejko commented Apr 27, 2024

I see that it's working as intended.

@alat-rights
Copy link
Owner

The flaky import isn't raising problems for me. What's the error that made you type: ignore it?

@alat-rights
Copy link
Owner

Do you want to make a separate PR with only the CI and formatting change?

Would probably be able to get that merged in quickly.

@Domejko
Copy link
Contributor Author

Domejko commented Apr 27, 2024

I have added ignore on flaky because at the moment tests can't be run locally so there is no reason (but only temporary) to install flaky and when it's not installed then pyright throws an error about it.

Sure I will just remove README.md from this PR and put it in separate one.

@alat-rights
Copy link
Owner

alat-rights commented Apr 27, 2024 via email

@alat-rights alat-rights merged commit 60de98d into alat-rights:master Apr 27, 2024
1 check passed
@Domejko
Copy link
Contributor Author

Domejko commented Apr 27, 2024

Because there are no local tests written yet, only with live API. Matter that we discussed in #10

@Domejko Domejko changed the title Style ci and readme update Style ci update Apr 27, 2024
@alat-rights
Copy link
Owner

alat-rights commented Apr 27, 2024

I see.

It sounds like you prefer to keep flaky out of your dev environment until you're able to run the tests.

I think for future contributors, it'd be good if PyRight catches something that breaks tests rather than not.

Also since we might have flaky tests that don't depend on Anthropic API in the future, I think it's reasonable for missing flaky to raise a problem.

If your problem with that line is purely local, I'd lean toward not type ignoring that line.

alat-rights added a commit that referenced this pull request Apr 27, 2024
@alat-rights
Copy link
Owner

alat-rights commented Apr 27, 2024

I rolled back that specific change for now. We can discuss more if you feel strongly!

Good job on the PR :)

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.

None yet

2 participants