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

Switch to Poetry, add dependabot and pre-commit for linting. Remove versioneer #603

Closed
wants to merge 1 commit into from
Closed

Conversation

orf
Copy link
Contributor

@orf orf commented Mar 7, 2022

POC for #602

I also refactored the tests to use fixtures, which I think is a lot more flexible and will allow us to split the gigantic test file up a bit.

Edit: I also refactored the moto3 server fixture to keep a persistent server running and use unique bucket names. This speeds up the tests a lot.

@martindurant
Copy link
Member

I am certainly in favour of precommit, and running automatic jobs locally as they do in CI. This should be made uniform across all projects.

I remain to be convinced by poetry...

@efiop , perhaps you have an opinion on applying this kind of thing here and for other fsspec projects?

@orf
Copy link
Contributor Author

orf commented Mar 8, 2022

I remain to be convinced by poetry...

The current setup.py, requirements.txt, test-requirements.txt, manifest.ini setup is definitely considered to be a legacy way of setting up a project now.

Poetry is really simple way to get a consistent project setup with no fuss. The lockfile also guarantees reproducible builds, and combined with a thing like Dependabot means that we'll have a fairly immediate way of knowing if one of our dependencies breaks us without breaking all existing builds. I like it a lot, and while it's not perfect it's probably as close as we've got so far to a simple, fast and consistent CLI (a-la cargo). It intergates well with IDEs, it's fast and it's sane: poetry run [cmd], poetry add [dependency] (--dev), poetry install is about all you really need.

All I can suggest is that we try it and see what happens, and we can always revert if you feel unhappy with it.

@orf
Copy link
Contributor Author

orf commented Mar 25, 2022

I've split out the test refactor into #611 - this is a nightmare to rebase and I think it has a great impact on test readability + speed.

I've split pre-commit out into #612.

@efiop
Copy link
Member

efiop commented Mar 30, 2022

I am certainly in favor of precommit, and running automatic jobs locally as they do in CI. This should be made uniform across all projects.

💯 Related to fsspec/community#6 I would start there and then propagate the template into particular projects.

@efiop
Copy link
Member

efiop commented Mar 31, 2022

Sorry, I don't have the capacity right now to start it myself, but I would definitely wait (or start) for the template to be introduced and not proceed with this PR here for now. Might be able to come back to that in the mid term future.

@martindurant
Copy link
Member

+1, let's aim to do this (and pre-commit.ci rather than a action! ) for the next release cycle in ~1month.

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.

3 participants