Skip to content

Update version in makefile. Returns proper version of pydo #144

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

Merged
merged 8 commits into from
Dec 9, 2022

Conversation

ChiefMateStarbuck
Copy link
Contributor

This PR update the version in the makefile that is used to generate the pydo client. pydo now returns the proper version.

The reformatting of extra files was due to regenerating the code.

>>> import pydo
>>> pydo.VERSION
'0.1.3'

danaelhe
danaelhe previously approved these changes Dec 8, 2022
Copy link
Member

@danaelhe danaelhe left a comment

Choose a reason for hiding this comment

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

👍

@andrewsomething
Copy link
Member

If we need to hard code this, can you update the documentation to include the need to update this before making a new release?

https://github.com/digitalocean/pydo/blob/main/CONTRIBUTING.md#releasing-pydo

@ChiefMateStarbuck
Copy link
Contributor Author

ChiefMateStarbuck commented Dec 8, 2022

If we need to hard code this, can you update the documentation to include the need to update this before making a new release?

https://github.com/digitalocean/pydo/blob/main/CONTRIBUTING.md#releasing-pydo

added a line to re-run make generate. this will update files as needed.
https://github.com/digitalocean/pydo/pull/144/files#diff-eca12c0a30e25b4b46522ebf89465a03ba72a03f540796c979137931d8f92055R104

4. Make a pull request with this change. It should be separate from PRs
containing chagnes to the library (including regenerated code).
5. *Once the version bump PR has been pushed*, tag the commit to trigger the
4. Run `make generate` to update the version in the codebase.
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't make bump_version update the version? I guess we just didn't follow our own instructions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the pydo.VERSION is pulled from this line. This is part of the generated code so it would be best to not ask contributors to mess with generated code files. Running make generate will update this line. One option is to add the generate command to the end of the bump_version command to make it one command that updates the version in ALL places in the codebase.

Copy link
Contributor

@scotchneat scotchneat left a comment

Choose a reason for hiding this comment

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

The challenge here is there are now two places that need to be updated:

  • the version property in pyproject.toml: which is managed by poetry and was configured as the source of truth when writing scripts/bumpversion.sh.
  • the VERSION variable in src/pydo/_version.py: which appears to be updated manually in this change

We need to make one of those values the source of truth and stick with it.

If you read options listed by the Python Packaging User Guide on Single-sourcing the package version, 1 describes a way to configure pyproject.toml to pull the value dynamically from the package variable.

Note: This will likely invalidate the current process of bumping the version using poetry

I think I prefer the option described in 5 using importlib.metadata as it can pull from the pyproject.toml file and we wouldn't have to change up the bump process at all.

import sys

if sys.version_info >= (3, 8):
    from importlib import metadata
else:
    import importlib_metadata as metadata

VERSION = metadata.version('pydo')

@ChiefMateStarbuck
Copy link
Contributor Author

ChiefMateStarbuck commented Dec 9, 2022

The challenge here is there are now two places that need to be updated:

  • the version property in pyproject.toml: which is managed by poetry and was configured as the source of truth when writing scripts/bumpversion.sh.
  • the VERSION variable in src/pydo/_version.py: which appears to be updated manually in this change

We need to make one of those values the source of truth and stick with it.

If you read options listed by the Python Packaging User Guide on Single-sourcing the package version, 1 describes a way to configure pyproject.toml to pull the value dynamically from the package variable.

Note: This will likely invalidate the current process of bumping the version using poetry

I think I prefer the option described in 5 using importlib.metadata as it can pull from the pyproject.toml file and we wouldn't have to change up the bump process at all.

import sys

if sys.version_info >= (3, 8):
    from importlib import metadata
else:
    import importlib_metadata as metadata

VERSION = metadata.version('pydo')

The VERSION in src/pydo/_version.py is managed automatically when re-running make generate. Once the version in pyproject.toml is updated, re-running the generator will update VERSION. I added a line in the contributing file per ASB's request

@scotchneat
Copy link
Contributor

Right on

@ChiefMateStarbuck ChiefMateStarbuck merged commit 1bf9be8 into main Dec 9, 2022
@ChiefMateStarbuck ChiefMateStarbuck deleted the proper-version branch December 9, 2022 16:41
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.

4 participants