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

Address some testing and packaging issues #255

Merged
merged 16 commits into from
Apr 28, 2022
Merged

Conversation

gipert
Copy link
Member

@gipert gipert commented Apr 25, 2022

Lots of new features: here's a short summary of the changes.

  • Enable GitHub Dependabot to keep GitHub actions automatically up to date. There will be a bot opening a pull request once in a while (I expect it to be very quiet) to update the GitHub action workflow file.
  • Add https://codecov.io configuration file (this will enable the corresponding bot) and upload coverage automatically with GitHub actions. For every pull request the Codecov bot will post a summary of the test coverage status and tell us whether the proposed changes are sufficiently tested. Let's see how noisy this is: we can temporary disable the bot while in this heavy-development phase
  • Enable the https://pre-commit.ci bot to automatically perform style checks through pre-commit: there will be now an additional check run for every commit ensuring that coding conventions are respected. The pre-commit.ci bot will automatically fix (wherever possible) style-related issues in every PR, by adding a commit. The list of checks to be performed are specified through the .pre-commit-config.yaml configuration file. At the moment I just enabled some basic (but extremely useful!) checks: trailing whitespace or newlines, dead symlinks, large files... Later we can also enable some python linters (like black or flake8) or static type checkers (mypy): this will be also extremely useful but requires a massive re-format of the existing code – we can do it once the refactoring is over
  • Update developer docs

See also #241 and #242.

@gipert gipert added tests Testing the package packaging Project packaging labels Apr 25, 2022
@gipert gipert self-assigned this Apr 25, 2022
@codecov-commenter
Copy link

codecov-commenter commented Apr 26, 2022

Codecov Report

❗ No coverage uploaded for pull request base (refactor@4935723). Click here to learn what that means.
The diff coverage is n/a.

@@             Coverage Diff             @@
##             refactor     #255   +/-   ##
===========================================
  Coverage            ?   21.10%           
===========================================
  Files               ?       20           
  Lines               ?     2421           
  Branches            ?        0           
===========================================
  Hits                ?      511           
  Misses              ?     1910           
  Partials            ?        0           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4935723...851f0c7. Read the comment docs.

@gipert gipert marked this pull request as ready for review April 26, 2022 11:37
@gipert
Copy link
Member Author

gipert commented Apr 26, 2022

@jasondet, @iguinn: can check whether this looks fine to you? The changes are 90% due to pre-commit hooks removing trailing whitespace/newlines, checking spelling and sorting python imports – they should be innocuous. I did not enable hooks that apply formatting rules yet, we may do that once the refactoring is over and we'll maybe have more tests.

The very cool thing about these hooks is that they will be run for each PR and will keep the code automatically formatted.

@iguinn: I configured a pre-commit hook that strips output & metadata field from Jupyter notebooks – should be a better way to handle the issue. I will cleanup setup.py in a future PR.

@gipert gipert requested review from jasondet and iguinn and removed request for jasondet April 26, 2022 12:45
@jasondet
Copy link
Collaborator

It looks good to me. I'm not going to go through and check the "viewed" box for all 89 files though ;)

I just merged the previous PR, can you fix the new conflicts?

@gipert
Copy link
Member Author

gipert commented Apr 28, 2022

Can this be merged? Objections?

@iguinn iguinn merged commit 0d74254 into legend-exp:refactor Apr 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
packaging Project packaging tests Testing the package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants