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

fix: update pypi package name #454

Merged
merged 13 commits into from May 26, 2020
Merged

fix: update pypi package name #454

merged 13 commits into from May 26, 2020

Conversation

skuruppu
Copy link
Contributor

Towards #449

@skuruppu skuruppu added the api: spanner Issues related to the googleapis/python-spanner-django API. label May 19, 2020
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label May 19, 2020
Copy link
Contributor

@odeke-em odeke-em left a comment

Choose a reason for hiding this comment

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

LGTM, thank you @skuruppu! Could you perhaps please also tag this issue #455 in your commit message to say Fixes #455? Thank you.

@odeke-em
Copy link
Contributor

I shall later on send docs updates to indicate the new name.

setup.py Show resolved Hide resolved
@odeke-em
Copy link
Contributor

odeke-em commented May 19, 2020 via email

Copy link
Contributor

@busunkim96 busunkim96 left a comment

Choose a reason for hiding this comment

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

I think this setup.py would benefit from having a few more fields:

  • url
  • python_requires (pip won't install the package if the Python version is incompatible)
  • long_description

See google-cloud-spanner's setup.py for comparison

setup.py Show resolved Hide resolved
setup.py Outdated Show resolved Hide resolved
@skuruppu
Copy link
Contributor Author

I think this setup.py would benefit from having a few more fields:

  • url
  • python_requires (pip won't install the package if the Python version is incompatible)
  • long_description

See google-cloud-spanner's setup.py for comparison

I made setup.py look similar to https://github.com/googleapis/python-spanner/blob/master/setup.py.


name = "django-google-spanner"
description = "Bridge to enable using Django with Spanner."
version = "2.2a0"
Copy link
Contributor Author

@skuruppu skuruppu May 19, 2020

Choose a reason for hiding this comment

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

I assume release-please will replace this version since this doesn't look like the version number it's proposing in #453

Copy link
Contributor

@timgraham timgraham May 20, 2020

Choose a reason for hiding this comment

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

That wouldn't be ideal. The README explains the typical versioning of database backends: "Use the version of django-spanner that corresponds to your version of Django. For example, django-spanner 2.2.x works with Django 2.2.y. (This is the only supported version at this time.)"

The specifics of the alternate proposal are unclear but I'd think the maintenance burden and user confusion about which version to use would be higher.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@timgraham thanks for that info. But see my comment. There are lots of existing django packages that are published by Google and they all consistently use the versioning scheme used by release-please.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, sem-var versioning is typical for other Django libraries that often support multiple versions of Django. Database backends are different. The best practice is to create separate release branches for each version of Django that's supported.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I spoke to @larkee and they explained that the manual releasing won't be so painful.

re: versions

I would prefer semver for consistency with other packages. It also makes it easier for folks to pin on this library (e.g., google-django-spanner>=0.1.0,<1.0.0

@timgraham do you have any thoughts on the issue of pinning releases when you have non-semver versioning?

Copy link
Contributor

Choose a reason for hiding this comment

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

If using Django 2.2.x, use django-google-spanner>=2.2,<3.0. I think django-google-spanner==2.2.* would be equivalent.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In that case, @timgraham or @odeke-em could you update the README to explain why the versioning is the way it is? Please include links to the Django README as a reference.

Copy link
Contributor

Choose a reason for hiding this comment

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

What I quoted is from the README in this repo.

@odeke-em
Copy link
Contributor

odeke-em commented May 19, 2020 via email

setup.py Outdated Show resolved Hide resolved
@odeke-em
Copy link
Contributor

odeke-em commented May 19, 2020 via email

@busunkim96
Copy link
Contributor

re: versions

I would prefer semver for consistency with other packages. It also makes it easier for folks to pin on this library (e.g., google-django-spanner>=0.1.0,<1.0.0 If you all decide on some other version schema please explain it clearly in the README. The release bot won't be able to figure out what the correct 'next' release is, so you will have to manually propose releases.

  1. Open a PR. In the body of the commit (or the squash merge commit), add 'Release-As: x.x."
feat: release 2.2a0

Release-As: 2.2.a0
  1. Wait for Release-Please to open a new PR.

@skuruppu
Copy link
Contributor Author

@busunkim96, for reducing the maintenance burden on my team, I don't want to deviate from the versioning done by other python packages in googleapis. We want to automate as much of the chores as possible.

@odeke-em, this is how other googeapis django releases are versioned (if you click through the search links).

setup.py Show resolved Hide resolved
Copy link
Contributor

@larkee larkee left a comment

Choose a reason for hiding this comment

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

LGTM

@larkee larkee merged commit 47154d1 into master May 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: spanner Issues related to the googleapis/python-spanner-django API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants