-
Notifications
You must be signed in to change notification settings - Fork 136
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
ref: Move build configuration from setup.py
to pyproject.toml
; apply various repo/CI refactors
#602
ref: Move build configuration from setup.py
to pyproject.toml
; apply various repo/CI refactors
#602
Conversation
@@ -112,7 +109,7 @@ def main(): # pylint: disable=too-many-branches,too-many-statements | |||
if not parsed.command: | |||
# print version info and exit - but only if no command was given | |||
print(f"linode-cli {VERSION}") | |||
print(f"Built off spec version {cli.spec_version}") | |||
print(f"Built from spec version {cli.spec_version}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't really relevant to the rest of these changes but I think this wording is a tiny bit less awkward 🙂
VERSION = version("linode-cli") | ||
except: | ||
VERSION = "building" | ||
VERSION = __version__ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be much more reliable than grabbing the version from the package itself.
@@ -34,13 +34,18 @@ jobs: | |||
# This is necessary as we want to ensure that version tags | |||
# are properly formatted before passing them into the | |||
# DockerFile. | |||
- name: Get CLI version |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I honestly have no clue how this worked in the last release given we removed the version
script beforehand. Regardless, I moved it over to a more reliable solution 🙂
@@ -2,6 +2,51 @@ | |||
requires = ["setuptools", "wheel", "packaging"] | |||
build-backend = "setuptools.build_meta" | |||
|
|||
[project] | |||
name = "linode-cli" | |||
authors = [{ name = "Akamai Technologies Inc.", email = "developers@linode.com" }] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a note, I swapped this from Linode
to Akamai Technologies Inc.
setup.py
to pyproject.toml
; drop Jenkinsfile and requirements.txtsetup.py
to pyproject.toml
; apply various repo/CI refactors
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work! The code base is much cleaner now. 🎉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice refactor! LGTM
📝 Description
This change makes the following build system improvements to the Linode CLI:
setup.py
in favor of setuptools inpyproject.toml
baked_version
construct in favor of injecting the version into a newversion.py
file.requirements.txt
andrequirements-dev.txt
in favor ofpyproject.toml
/etc/bash_completion.d
version
script (which was previously removed)✔️ How to Test
The following test steps assume you have pulled down this PR locally.
Testing Workflows
For convenience I tested all workflows on the
main
branch of my fork; they can be viewed here:Testing the Build & Install Process
v0.0.0.dev
:v0.0.0.dev
:Testing the Dockerfile Changes
Create a read-only GitHub Personal Access Token and export it under the
GITHUB_TOKEN
environment variable.Run the following command to build the CLI OCI image:
Unit Testing
Integration Testing