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

Modernize repo #304

Merged
merged 9 commits into from
Jan 29, 2024
Merged

Modernize repo #304

merged 9 commits into from
Jan 29, 2024

Conversation

leplatrem
Copy link
Contributor

  • Replaced setup.cfg and setup.py by pyproject.toml
  • Replaced flake8 by ruff
  • Use requirements.in instead of list in setup.py
  • Replaced manually created requirements.txt by pip-compile
  • Remove dev-requirements.txt file and use extra dependencies instead
  • Use version from git tag (setuptools-scm)
  • Publish to Pypi on git tag
  • Add config to categorize changes in autogenerated changelog
  • Add Github actions to dependabot
  • Update release instructions
  • Rename master to main
  • Replace tox with GH actions
  • Move tests out of package
  • Move package to src
  • Force labels on pull-requests (for better changelog)

Things lost with this PR:

  • No more changelog (now on GH releases)
  • Description on Pypi does not contain changelog anymore

setup.py Show resolved Hide resolved
@alexcottner
Copy link

alexcottner commented Jan 26, 2024

FAIL Required test coverage of 100% not reached. Total coverage: 99.00%\

IMO, striving for a 100% code coverage metric makes us focus on the wrong things. As long as our business logic is covered (which should be >80% in projects that aren't just all IO), I'm happy.


# Run chosen subcommand.
if args.which == "dump":
if args.full:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: It always feels like I should have another function when my if has more than one thought in it. ex:

if args.which == "dump"
  doDump(...)
elif args.which == "load"
  doLoad(...)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

This is a bit out of scope, the project probably has a lot more to refactor and polish

@alexcottner
Copy link

Looks good, we just need to fix the CI checks

@leplatrem leplatrem merged commit a31a441 into main Jan 29, 2024
4 of 5 checks passed
@leplatrem leplatrem deleted the modernize-repo branch January 29, 2024 12:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants