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
Conversation
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 shall later on send docs updates to indicate the new name. |
4b50bd7
to
d2edb9b
Compare
What’s the rationale for removing it? It is necessary for folks who’ll be
using this package to install once and not have to manually install
google-cloud as well. I could be wrong though, and if there is another way
to do it then sure.
…On Mon, May 18, 2020 at 5:23 PM skuruppu ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In setup.py
<#454 (comment)>
:
> @@ -10,12 +10,11 @@
install_requires = [
'sqlparse >= 0.3.0',
- 'google-cloud >= 0.34.0',
@odeke-em <https://github.com/odeke-em>, @busunkim96
<https://github.com/busunkim96> asked me to remove this dependency. Hope
that's ok with you.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#454 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABFL3V4PL77CAD2QSQY2FUTRSHGR5ANCNFSM4NEQ4CIA>
.
|
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 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" |
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 assume release-please will replace this version since this doesn't look like the version number it's proposing in #453
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.
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.
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.
@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.
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.
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.
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 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?
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.
If using Django 2.2.x, use django-google-spanner>=2.2,<3.0
. I think django-google-spanner==2.2.*
would be equivalent.
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.
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.
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.
What I quoted is from the README in this repo.
If possible I’d highly encourage we stick to the 2.2a0 format that
currently exists because it contains what version of Django (2.2) is
supported but also what version of the release is in (a0)
…On Mon, May 18, 2020 at 9:57 PM skuruppu ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In setup.py
<#454 (comment)>
:
>
-install_requires = [
+# Package metadata.
+
+name = "django-google-spanner"
+description = "Bridge to enable using Django with Spanner."
+version = "2.2a0"
I assume release-please will replace this version since this doesn't look
like the version number it's proposing in #453
<#453>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#454 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABFL3VZLJDDPPT3CNDG2A3DRSIGTHANCNFSM4NEQ4CIA>
.
|
Nope, it is meant for Python 3.5 and above.
…On Tue, May 19, 2020 at 9:59 AM Bu Sun Kim ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In setup.py
<#454 (comment)>
:
> @@ -39,4 +58,6 @@
'Framework :: Django',
'Framework :: Django :: 2.2',
],
+ extras_require=extras,
+ python_requires=">=2.7,!=3.0.*,!=3.1.*,!=3.2.*,!=3.3.*",
@odeke-em <https://github.com/odeke-em> Is this library compatible with
2.7?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#454 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABFL3V3MJNOQNXKB4LW2CXLRSK3G5ANCNFSM4NEQ4CIA>
.
|
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.,
|
@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). |
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.
LGTM
Towards #449