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

Add mypy config, CI, and fix type #54

Closed
wants to merge 3 commits into from
Closed

Add mypy config, CI, and fix type #54

wants to merge 3 commits into from

Conversation

mkly
Copy link
Member

@mkly mkly commented Jan 31, 2024

  • Update pyproject.toml config for jq
  • Add mypy check to CI
  • Fix small type issue so mypy passes

* Add mypy.ini config for `jq`
* Add mypy check to CI
* Fix small type issue so mypy passes
Copy link

github-actions bot commented Jan 31, 2024

MLCommons CLA bot All contributors have signed the MLCommons CLA ✍️ ✅

@mkly
Copy link
Member Author

mkly commented Jan 31, 2024

As mentioned in the previous PR, 100% okay we would rather not add this. Just easy stuff to do while I'm learning the code base.

@wpietri
Copy link
Contributor

wpietri commented Feb 1, 2024

Sorry, I'm not getting the intent of this PR. What are you looking to improve?

@mkly
Copy link
Member Author

mkly commented Feb 1, 2024

Apologies, I should have provided more context. As you probably noticed, I added the black check to CI. When doing that on newhelm it was suggested that I also add a check with mypy. I was applying that check here and the changes relate to getting the mypy test to pass locally and in CI.

I can certainly understand if you would prefer not to add this, so if you would rather close this out, no problem at all.

@wpietri
Copy link
Contributor

wpietri commented Feb 1, 2024

Let's all discuss it. Putting a tool in CI shifts it from being a supportive relationship to a controlling one. I'm grudgingly OK with that for black, but I have more concerns about mypy, especially on a project that's still pursuing product-market fit.

@mkly
Copy link
Member Author

mkly commented Feb 1, 2024

Closing as per morning discussion. Thanks.

@mkly mkly closed this Feb 1, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Feb 1, 2024
@mkly mkly deleted the mkly/mypy-1 branch February 2, 2024 00:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants