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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

A few project health suggestions #161

Closed
9 tasks done
antonagestam opened this issue Apr 16, 2021 · 5 comments 路 Fixed by #168
Closed
9 tasks done

A few project health suggestions #161

antonagestam opened this issue Apr 16, 2021 · 5 comments 路 Fixed by #168

Comments

@antonagestam
Copy link
Contributor

antonagestam commented Apr 16, 2021

Hi again 馃憢

After working a little bit with the project I stumbled over a few things that I felt could be nice to address. Since I don't want to overwhelm anyone I thought it might be a good idea to collect them in a shared issue to see which ones there are interest for.

  • Update release.sh to use pre-commit. Replace release.sh with a Github Actions job.
  • Move package into a src/ directory.
  • Move tests out of package. It seems more common to keep tests in a separate tests package outside of the project package itself.
  • Move packaging config to [metadata] in setup.cfg pyproject.toml.
  • Harmonize py.test vs pytest in docs, scripts and tox. Perhaps a make target would be a good idea? These should all use tox, CI can use TOX_ENV.
  • Test with -W error so that builds fail if there are warnings.
  • Test with --doctest-modules, and make all current doctests compliant.
  • Test with --doctest-glob='*.rst', and make all examples in docs runnable on their own.
  • Define test requirements with extras instead of requirements.txt files.
@Stranger6667
Copy link
Collaborator

Hi :) Thank you for bringing these points!

  • I often think about moving it completely to CI, like in django-money (maybe with updating to build instead of pep517) but reusing pre-commit in that script is a great idea! I, personally, try to avoid building & uploading packages from my local machine (mostly to ensure a clean build environment)
  • 馃憤. Some folks like to have tests for downstream packages, but generally, I don't find having tests as a part of the distribution sufficient for testing, as it usually requires some infrastructure, build environment, etc. So, IMO, it is ok to move it out and also not include it in the distributed package; Or do you mean to move it out and still include it in the distribution?
  • Hm, what do you think about pyproject.toml? Are there some limitations?
  • Yep 馃憤 What about having tox jobs only?
  • 馃憤
  • 馃憤
  • 馃憤

Additionally, I'd like to also revert this commit and have src as per the reasons described here or here. What do you think about it?

@antonagestam
Copy link
Contributor Author

Awesome! I might not be able to get the ball rolling on all of these immediately, but having a shared vision like this seems very promising.

I often think about moving it completely to CI, like in django-money

Oh, that's a nice one. I have similar one in phantom-types but it relies on pushing "release-request" tags. Using the actual release feature looks much nicer.

So, IMO, it is ok to move it out and also not include it in the distributed package; Or do you mean to move it out and still include it in the distribution?

Cool, yes moving it out and not including it is what I meant. I read this discussion some time back and just figured that was dying practice ;) A related thing is perhaps to define dev and test requirements as extras, and let tox define -e .[dev,test] as requirement. I need to read up on that article about src though, which also seems related to this topic, I'll add it to the list too.

pyproject.toml sounds good, I recall there being issues in pip but I think they are resolved now, that's the reason I haven't moved in that direction myself yet.

What about having tox jobs only?

That makes sense, let's skip make. The Github Actions matrix could probably be configured with TOX_ENV, right?

@Stranger6667
Copy link
Collaborator

Awesome! I might not be able to get the ball rolling on all of these immediately, but having a shared vision like this seems very promising.

I am happy to hear that :)

Cool, yes moving it out and not including it is what I meant. I read this discussion

The link is malformed, could you, please share the right one? I'd like to read it as well :)

A related thing is perhaps to define dev and test requirements as extras, and let tox define -e .[dev,test]

Yep, extras sound good to me.

That makes sense, let's skip make. The Github Actions matrix could probably be configured with TOX_ENV, right?

I believe this one is resolved by tox-gh-actions :)

@antonagestam
Copy link
Contributor Author

@Stranger6667 Oops, this is the link I meant to refer to: pytest-dev/pytest#5534

@Stranger6667
Copy link
Collaborator

Thanks!

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 a pull request may close this issue.

2 participants