-
Notifications
You must be signed in to change notification settings - Fork 30
Docs: updated README.rst
file
#563
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
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.
Changes LGTM, just a few nits. I don't think we should check in any "under construction" pages though.
README.rst
Outdated
} | ||
} | ||
|
||
- In order to retrieve the Cloud Spanner credentials from a JSON file, ``credentials_uri`` parameter can also be supplied in the ``OPTIONS`` field: |
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 order to retrieve the Cloud Spanner credentials from a JSON file, ``credentials_uri`` parameter can also be supplied in the ``OPTIONS`` field: | |
- In order to retrieve the Cloud Spanner credentials from a JSON file, the ``credentials_uri`` parameter can also be supplied in the ``OPTIONS`` field: |
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.
Done!
README.rst
Outdated
@@ -27,7 +42,7 @@ To install from PyPI: | |||
pip3 install django-google-spanner | |||
|
|||
|
|||
To install from source: | |||
To install from the source: |
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.
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.
👍 Done!
README.rst
Outdated
constraints | ||
<https://github.com/googleapis/python-spanner-django/issues/313>`__. | ||
Spanner does not support ``ON DELETE CASCADE`` when creating foreign-key | ||
constraints, so these are not supported in ``django-google-spanner``. |
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.
constraints, so these are not supported in ``django-google-spanner``. | |
constraints, so this is not supported in ``django-google-spanner``. |
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.
👍 Done!
README.rst
Outdated
|
||
Check constraints aren't supported | ||
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
|
||
Spanner doesn't support ``CHECK`` constraints so one isn't created for | ||
Spanner does not support ``CHECK`` constraints, so one isn't created for |
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.
"Doesn't" get expanded by "isn't" doesn't?
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.
👍 Done!
@@ -189,13 +192,11 @@ Spanner doesn't support it. For example: | |||
Schema migrations | |||
~~~~~~~~~~~~~~~~~ | |||
|
|||
Spanner has some limitations on schema changes which you must respect: | |||
There are some limitations on schema changes to consider: |
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.
👍
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.
Looks like this one has already been fixed.
docs/django-spanner-usage.rst
Outdated
Usage | ||
===== | ||
|
||
[this page is under construction] |
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 it's better to avoid checking "under construction" pages in, and just add them once they're written.
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.
Done!
README.rst
Outdated
To use this library you'll need a Google Cloud Platform project with the Cloud | ||
Spanner API enabled. See the `Cloud Spanner Python client docs | ||
Using this library requires a Google Cloud Platform project with the Cloud | ||
Spanner API enabled. See the Spanner' Python Client `documentation |
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.
Spanner API enabled. See the Spanner' Python Client `documentation | |
Spanner API enabled. See the Spanner Python client `documentation |
I'm no usage expert, but I think "Spanner" and "Python" are capitalized here as separate terms -- not as part of "Spanner Python Client" -- and "client" can be lowercase.
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.
👍 Done.
Changelog:
README.rst
file to reflect the latest state of development;connection-usage.rst
file.