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

packaging: Move version definition to setup.cfg #493

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

zmc
Copy link

@zmc zmc commented May 5, 2022

An unfortunate side-effect of defining the version inside a project's
main module is that dependency issues can break installation. In this
project, even though pyyaml and python-dotenv are listed in
install_requires, installation would fail if either were not present,
because the build process requires importing that module.

Signed-off-by: Zack Cerza zack@cerza.org

An unfortunate side-effect of defining the version inside a project's
main module is that dependency issues can break installation. In this
project, even though pyyaml and python-dotenv are listed in
install_requires, installation would fail if either were not present,
because the build process requires importing that module.

Signed-off-by: Zack Cerza <zack@cerza.org>
@muayyad-alsadi
Copy link
Collaborator

I would love for the podman-compose.py to be a standalone script too.

I don't want to add unnecessary dependencies

@@ -35,8 +41,6 @@
import yaml
from dotenv import dotenv_values

__version__ = "1.0.4"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it was fine if both setup.cfg and podman_compose.py duplicated the version.

The dependency problems is a valid justification to do so. The downside is that these versions can get out of sync. However this can be addressed by having CI run a simple regex based test that extracts versions from files and compares them.

Copy link
Collaborator

@p12tic p12tic left a comment

Choose a reason for hiding this comment

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

Best to duplicate the version and add a test to ensure that the versions are synchronized.

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.

3 participants