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 issue 59 (refactor build system) #85

Merged
merged 28 commits into from
Mar 31, 2024
Merged

Address issue 59 (refactor build system) #85

merged 28 commits into from
Mar 31, 2024

Conversation

matthewcarbone
Copy link
Collaborator

@matthewcarbone matthewcarbone commented Feb 14, 2024

Current WIP, I'll explain the changes I'm making as I go.

Below is my summary of changes

Refactor semantic versioning

Status: ✅ done

I have spent a while trying to figure out a robust way of getting the tag of the commit to be automatically swapped in for the __version__ dunder attribute in the __init__.py files. I have failed to find a way to do this without actually rewriting part of the file itself, so that's what I did. It's ugly, but it works! I think I might even write a GH action for it at some point.

Modernize build system

Status: ✅ done

This includes deprecating setup.py in favor of pyproject.toml and also using a more modern build system like flit.

Note that the deploy workflow only runs on pushes and when there is a tagged commit!

Refactor .github/workflows system

Status: ✅ done

Just trying to clean this up a bit, and refactor the workflow files so they're much easier to read.

@matthewcarbone matthewcarbone self-assigned this Feb 14, 2024
@matthewcarbone matthewcarbone marked this pull request as draft February 14, 2024 15:46
@matthewcarbone
Copy link
Collaborator Author

Note all builds are going to break for a while as I purposefully removed __version__ for now.

@codecov-commenter
Copy link

codecov-commenter commented Feb 15, 2024

Codecov Report

Attention: Patch coverage is 81.81818% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 95.47%. Comparing base (72c7255) to head (7d8156b).

Files Patch % Lines
gpax/_version.py 66.66% 2 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #85      +/-   ##
==========================================
+ Coverage   95.24%   95.47%   +0.22%     
==========================================
  Files          55       54       -1     
  Lines        4247     4239       -8     
==========================================
+ Hits         4045     4047       +2     
+ Misses        202      192      -10     
Flag Coverage Δ
unittests 95.47% <81.81%> (+0.22%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@matthewcarbone
Copy link
Collaborator Author

@ziatdinovmax I've changed quite a few files so your eyes on the latest commit would be great. Please don't hesitate to ask any questions!

The strangest, but in my opinion, nicest, difference is that you no longer have to manually update __version__ in your __init__.py. It's rewritten for you using a grep command and dunamai at build time. Any tagged release will do this and will then run your pypi deployment environment (which I cannot see, but you should set in a way that requires your explicit approval to release). I have also set the pypi deployment target to their test server so you can play around with this without worrying about deploying a problematic build to production.

@ziatdinovmax
Copy link
Owner

@matthewcarbone - thanks! I will review it shortly.

@matthewcarbone matthewcarbone marked this pull request as ready for review February 26, 2024 21:08
@matthewcarbone
Copy link
Collaborator Author

@ziatdinovmax I'll need to rebase and force push again. I also might have a slightly better way to do this, though it'll be similar in spirit. Going to convert back to a draft temporarily. I'll let you know when it's done!

@matthewcarbone matthewcarbone marked this pull request as draft February 29, 2024 17:58
@matthewcarbone
Copy link
Collaborator Author

@ziatdinovmax sorry for the main -> mc/issue-59 merge. The rebase was just too ugly.

@matthewcarbone
Copy link
Collaborator Author

matthewcarbone commented Mar 1, 2024

One minor important change. To handle dynamic versioning, there is now

# gpax/__init__.py
from gpax._version import __version__

and

# gpax/_version.py
try:
    import dunamai as _dunamai
    __version__ = _dunamai.Version.from_any_vcs().serialize()
    del _dunamai
except ImportError:
    __version__ = "dev"

The above is used when dealing with local copies. I.e., if someone does git clone on the GPax repo, the version displayed will be that in their local development copy.

However, when actually building and deploying to PyPI, the following script is run.

# scripts/build_project.sh
pip install flit~=3.7
pip install dunamai==1.19.2
echo "__version__ = '$(dunamai from any --style=pep440 --no-metadata)'" >gpax/_version.py
flit build

This dynamically, at build time, rewrites gpax/_version.py with the pep440-styled version of the code. In the case of deployed pushes (i.e., tagged pushes) this will precisely be the tag that we use in vcs! For instance, in my most recent untagged local test build, the generated tarball is gpax-0.1.8.post37.dev0.tar.gz. But if I tag it, dunamai detects the tag and applies that instead.

Anyways, I think everything is pretty clean now. Let me know if you have any questions!

Just in case someone using GPax as a dev copy does not have dunamai
installed, GPax will now gracefully fail to detect the version
and will simply use "dev" as the version name.
@matthewcarbone matthewcarbone marked this pull request as ready for review March 1, 2024 20:27
@matthewcarbone
Copy link
Collaborator Author

@ziatdinovmax I think this is ready for your review at this point. Please let me know if you have any questions, and I think we should test the build/deploy once on the PyPI test server before launching it live!

scripts/LICENSE Outdated Show resolved Hide resolved
@ziatdinovmax ziatdinovmax merged commit c1f2a10 into main Mar 31, 2024
6 checks passed
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 this pull request may close these issues.

Quality-of-life suggestions for release, CI, semantic versioning, etc.
3 participants