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

Infrastructure refresh #129

Merged
merged 29 commits into from
May 6, 2023
Merged

Infrastructure refresh #129

merged 29 commits into from
May 6, 2023

Conversation

bjlittle
Copy link
Member

@bjlittle bjlittle commented Apr 27, 2023

🚀 Pull Request

Description

Okay... firstly, apologies 🙏

This PR is a bit of a bloater... I had some spare cycles and decided spend them by just "going-for-it":tm: and refresh all the infrastructure of nc-time-axis in a oner to modernize it before the technical debt caught up with us.

I've been a bit opinionated in some places, but overall I don't think there is anything too controversial within this PR... and I have tried to theme the commits. Anyways, the effort is now sunk, so there you go; it's all about permission and forgiveness 😉

In summary, this PR:

  • drops cirrus-ci and migrates to GitHub Actions (GHA) 🥳
    • automates citation file checking (ci-citation)
    • automates explicit conda lock file generation (*.lock) from top-level *.yml environment files (ci-locks) triggered on a cron
    • automates MANIFEST.in checking for the sdist (ci-manifest)
    • automates pytest over supported versions of python (ci-tests) with codecov test coverage reporting
    • automates the building, testing and publishing to PyPI of our sdist and binary wheels (ci-wheels)
  • adopts the workflow of leveraging pre-commit.ci and our hooks, rather than explicitly running the hooks ourselves in CI
  • enables dependabot to automate GHA version updates, so that we easily keep pace with the community
  • makes nc-time-axis citable (contributors please pile in and add your ORCID references)
  • refreshes the pre-commit hooks, most notably by adopting codespell to automate spell checking of the codebase and documentation
  • reconciles the MANIFEST.in with reality
  • pimps the README.md
  • adopts PEP621 by migrating the setup.cfg to the pyproject.toml
  • moves the nc-time-axis codebase under the src directory
  • co-locates both conda YAML and pip core and optional dependencies under the requirements directory (hopefully a better hint/reminder for devs to keep them aligned)
  • I also added the CODECOV_TOKEN, TEST_PYPI_API_TOKEN and PYPI_API_TOKEN as GHA secrets, as required by the GHAs

- Only the binary wheels are tested, not the sdist. However, the wheel is built from the sdist using pypa/build and their recommended workflow.

Yeah, I think that just about covers it. Soz. 😊

Closes #104

@bjlittle
Copy link
Member Author

bjlittle commented Apr 27, 2023

There is something slightly odd going on here... I've never quite seen it happen before.

The GHA CI is being triggered and executed on my forked repo development branch, but not here on the PR.

Thoughts anyone?

@bjlittle bjlittle closed this Apr 27, 2023
@bjlittle bjlittle reopened this Apr 27, 2023
@bjlittle
Copy link
Member Author

bjlittle commented Apr 27, 2023

@rcomer and @spencerkclark

Okay, just to be completely transparent here, I circumvented the branch protection on main to force push and undo the last commit resulting from the merge of the test PR #132.

This is pretty exceptional, however the GHA CI workflows appear to be now up and running.

I'm going to push a follow-on change to this PR branch to undo commit f61c2c7, which was reactive to the GHA oddness that we were experienceing.

Hopefully, the CI cogs will spin as expected on this PR...

@codecov-commenter
Copy link

codecov-commenter commented Apr 27, 2023

Codecov Report

Patch coverage has no change and project coverage change: +4.25 🎉

Comparison is base (8be5d69) 93.10% compared to head (e44c066) 97.36%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #129      +/-   ##
==========================================
+ Coverage   93.10%   97.36%   +4.25%     
==========================================
  Files           1        7       +6     
  Lines         203      644     +441     
  Branches       50        0      -50     
==========================================
+ Hits          189      627     +438     
- Misses          7       17      +10     
+ Partials        7        0       -7     
Impacted Files Coverage Δ
src/nc_time_axis/__init__.py 96.09% <ø> (ø)
src/nc_time_axis/tests/integration/test_plot.py 99.09% <ø> (ø)
...c_time_axis/tests/unit/test_AutoCFTimeFormatter.py 96.15% <ø> (ø)
...rc/nc_time_axis/tests/unit/test_CFTimeFormatter.py 100.00% <ø> (ø)
...c/nc_time_axis/tests/unit/test_CalendarDateTime.py 97.29% <ø> (ø)
...c_time_axis/tests/unit/test_NetCDFTimeConverter.py 99.41% <ø> (ø)
...time_axis/tests/unit/test_NetCDFTimeDateLocator.py 94.11% <ø> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Member

@spencerkclark spencerkclark left a comment

Choose a reason for hiding this comment

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

Well this is all pretty spectacular. Amazing to see nc-time-axis get this kind of treatment! All these new workflows / cleanups are great. Many thanks for putting this together @bjlittle. Just a couple suggestions on some places to update the documentation.

Also please give yourself credit for this in the release notes, and if you don't mind, maybe add a note for @rcomer's work in #128 as well!

requirements/docs.yml Show resolved Hide resolved
.github/workflows/ci-wheels.yml Show resolved Hide resolved
@bjlittle
Copy link
Member Author

bjlittle commented Apr 30, 2023

@spencerkclark Okay, over to you 👍

Note that, I ended up creating a common_links.inc file to capture common URLs, which hopefully makes the docs a wee bit easier going forwards. I also made some minor consistency changes to the docs e.g., title capitalsation, use of common links et al.

Hopefully I've not broken anything, but do take a peek and ensure that's the case.

Thanks again for taking the time to review this PR. Above and beyond.

Cheers mate 🍻

Copy link
Member

@spencerkclark spencerkclark left a comment

Choose a reason for hiding this comment

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

Looks good -- thanks for the additional cleanups to the documentation!

docs/release-notes.rst Outdated Show resolved Hide resolved
Co-authored-by: Spencer Clark <spencerkclark@gmail.com>
@bjlittle
Copy link
Member Author

bjlittle commented May 6, 2023

@spencerkclark Back to you 👍

@spencerkclark spencerkclark merged commit abc8f6c into SciTools:main May 6, 2023
@spencerkclark
Copy link
Member

In it goes!

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.

Add citation information?
4 participants