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

chore: start using hatch-vcs for version numbers #3007

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

jpivarski
Copy link
Member

No description provided.

@jpivarski
Copy link
Member Author

This will have to wait until after 2.6.1. We can use 2.6.2rc1 as a practice-run, like Uproot 5.2.2rc1.

@agoose77
Copy link
Collaborator

agoose77 commented Feb 5, 2024

The issue here is that the default GitHub checkout includes a single commit. That's not sufficient to deduce the version. We might be able to do something smart and download full-history only when the current workflow was triggered by a non-tag event (and filter that tag to ensure it's a ^v\d+ prefixed value. We'll need to update all of our GH workflows.

@jpivarski
Copy link
Member Author

jpivarski commented Feb 5, 2024

and filter that tag to ensure it's a ^v\d+ prefixed value

All of the tags in this git repo match one of three patterns:

  • v\d+\.\d+\.\d+(rc\d+)? for regular releases
  • awkward-cpp-\d+*
  • \d+\.\d+\.\d+(rc\d+)? for regular releases before 1.9.0rc12

It would be fine to consider only the first case.

@jpivarski jpivarski added the pr-inactive A pull request that hasn't been touched in a long time label Mar 20, 2024
@matthewfeickert
Copy link
Member

The issue here is that the default GitHub checkout includes a single commit. That's not sufficient to deduce the version. We might be able to do something smart and download full-history only when the current workflow was triggered by a non-tag event (and filter that tag to ensure it's a ^v\d+ prefixed value. We'll need to update all of our GH workflows.

How long does it take for GitHub to clone the entire repo history? It is for sure wasteful, but if it is less than 10 seconds is it that much of a delay in dev flow on PRs?

@jpivarski
Copy link
Member Author

How long does it take for GitHub to clone the entire repo history? It is for sure wasteful, but if it is less than 10 seconds is it that much of a delay in dev flow on PRs?

Personally, I'm not at all concerned with the time needed to clone the entire repo. It's much less than the time needed to run all of the tests.

@agoose77
Copy link
Collaborator

agoose77 commented Apr 12, 2024

I delved into this again, and it looks like there's still no easy way to avoid fetch-depth: 0.

Normally I'm one to push for plugins, but taking a step back I think it might not be a needed change. My feeling is that we want to make the release workflows simpler and less error prone. This PR would make that easier by detecting the version set by a new Release in the GitHub UI.

I wonder if it would be "better" to invert it, and have a workflow that creates a draft release when the version is bumped. Some potential benefits:

  • We can pre-populate the changelog (using our existing system, or something like github-activity)
  • Simpler / faster CI (no need to update fetch depths, although speed isn't a big problem right now)
  • Local development is simpler - versions are easily deduced from the source

Honestly, I'm biased; I don't love VCS versioning because it adds abstraction to something that I find useful to read from the source files. I'm happy to write the CI infrastructure to do this release work, but if @jpivarski you'd prefer the VCS-based solution, I won't keep pushing this alternative! I can push this PR over the line in that case.

@jpivarski
Copy link
Member Author

Until this year, I was against VCS because I wanted to have control over the version numbers; I wanted the version number to be what I set it to in a file.

However, as releases got more routine, I've been sold on streamlining the process. As a bonus, if someone checks out the code with git and builds it locally, the version number is different from any official release, which is useful information to consider in bug reports.

Then the blocker was that Awkward has a C++ part that doesn't integrate well with hatch-vcs. But now all of that is separated out into awkward-cpp, so in principle, it should work. That's why I started this PR. I don't really understand why it failed, but I haven't looked very deeply into it.

I'm a little worried that our suite of GitHub Actions workflows is too complex, in the sense that I don't understand them well enough to fix problems if they go awry. The tasks that I know they do are:

  • plain old CI testing (test.yml)
  • plain old CD deployment, for the pure Python awkward (deploy.yml)
  • deploying awkward-cpp, which is always invoked manually (deploy-cpp.yml)
  • checking to see if awkward-cpp is out of date and we need a new one (needs-cpp-release.yml)
  • deploying nightly wheels to Scientific Python's space on the Anaconda Cloud (upload-nightly-wheels.yml)
  • checking that the PR title, and eventual main commit, is formatted properly (semantic-pr-title.yml)

After that, it starts to get hazier: documentation is built and deployed in PR mergers, and it's built and put in a staging area on AWS for PR commits (a very useful feature when writing documentation). Those are probably docs.yml and docs-version.yml. Wheel-building gets tested separately from the basic CI, and that's build-wheels.yml, packaging-test.yml, or both. What remains is coverage.yml, lint.yml, and header-only-test.yml; I can guess what they do from their names, but I don't know when they run.


That was a long story—but if the thing that's holding this back is a formality, especially if it's one that complicates the workflows, rather than making them simpler, then I'd be on the side of removing that formality and adding standard hatch-vcs.

@jpivarski
Copy link
Member Author

At the least, I can get this PR up to date with main.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-inactive A pull request that hasn't been touched in a long time
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants