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

proposal: add more CI tooling for type safety, formatting, linting, and test coverage #81

Open
dpaiton opened this issue Nov 27, 2024 · 1 comment

Comments

@dpaiton
Copy link

dpaiton commented Nov 27, 2024

I'd like to add the following processes to the sparsecoding CI:

  • black
    • code formatter; has nothing to do with syntax, but strictly enforces opinionated and unified code structure
  • codecov
    • utility to display on our README how much test coverage we have; this is a great way to shame developers into writing more tests
  • pylint
    • this can work alongside flake8, but should probably replace it due to high amount of overlap in utility. It is slower, but more comprehensive and configurable than flake8. Given the size of the repo, I think comprehensive is more important than speed.
  • pyright
    • python has been steadily improving their type system, and generally encourages using it for new projects. I also am a huge advocate for typing. This is the standard type checker.

each of these have pros & cons, but collectively my argument for them is that "guardrails" are really valuable when you're working on a code base with a large team, or in an open-source setting. Including these sorts of checks in CI forces PR code to adhere to standards, instead of the PR reviewer having to do it.

Of them, the likely two most controversial are pyright & pylint.

Pylint replaces flake8. I don't have super strong opinions on this; if y'all really like flake8 then lets stick with it. We could also consider the young gun ruff which has fewer checks, is less configurable, but is crazy fast and cool bc it's written in rust 😎.

pyright is a philosophy change in the repo. Existing non-typed code can stay (for now), but setting up pyright encourages us to maintain typing going forward, and improving typing on existing code as we develop. I am a big proponent of this -- I think it helps you find bugs way faster, it improves readability & onboarding, and it is the direction the language is going in general. Python releases, peps, etc are focusing a lot of energy on type safety.

I will take the steps to implement these if we have a consensus.

@dpaiton dpaiton changed the title proposal: Add more CI tooling for formatting, linting, and test coverage proposal: add more CI tooling for formatting, linting, and test coverage Nov 27, 2024
@dpaiton dpaiton changed the title proposal: add more CI tooling for formatting, linting, and test coverage proposal: add more CI tooling for type safety, formatting, linting, and test coverage Nov 27, 2024
@belsten
Copy link
Collaborator

belsten commented Nov 28, 2024

I like the idea of trying Pylint. Hopefully it'll help us identified more issues before they're merged. And I don't see much drawback as the repo is small.

Pyright also seems great. I like typing and it really can make the code much more readable and robust. It would also make a good project for someone new to the repo to update the codebase with typing as well.

This gets a 👍 from me. @9q9q @HarrisonSantiago @ivanov what do you think?

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

No branches or pull requests

2 participants