chore: complete migration of django-google-spanner to librarian#16748
chore: complete migration of django-google-spanner to librarian#16748chalmerlowe merged 2 commits intogoogleapis:mainfrom
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request enables generation for the django-google-spanner library, updates its repository metadata, and replaces the documentation include with a comprehensive README. Review feedback identifies several syntax issues in the new documentation, including incorrect reStructuredText link formatting, the use of Markdown link syntax instead of RST, and a lowercase boolean literal in a Python code example.
| <https://docs.djangoproject.com/en/3.2/>`_, and `Django 4.2 | ||
| <https://docs.djangoproject.com/en/4.2/>`_. | ||
| Both versions are long-term support (LTS) releases for the | ||
| `Django project<https://www.djangoproject.com/download/#supported-versions>_`. |
There was a problem hiding this comment.
The reStructuredText link syntax is incorrect. It is missing a space before the opening angle bracket for the link text. Additionally, using a double underscore __ is preferred for anonymous inline links to match the style used elsewhere in this file.
| `Django project<https://www.djangoproject.com/download/#supported-versions>_`. | |
| Django project <https://www.djangoproject.com/download/#supported-versions>__. |
| 'PROJECT': '$PROJECT', | ||
| 'INSTANCE': '$INSTANCE', | ||
| 'NAME': '$DATABASE', | ||
| 'RANDOM_ID_GENERATION_ENABLED': false, |
There was a problem hiding this comment.
In Python, the boolean literal for false is False (capitalized). Since this is inside a .. code:: python block, it should follow Python syntax.
| 'RANDOM_ID_GENERATION_ENABLED': false, | |
| 'RANDOM_ID_GENERATION_ENABLED': False, |
|
|
||
| Django version 4.2 and higher by default supports transactions in autocommit mode. | ||
| A transaction is automatically started if you define an | ||
| [atomic block](https://docs.djangoproject.com/en/4.2/topics/db/transactions/#controlling-transactions-explicitly). |
There was a problem hiding this comment.
This line uses Markdown link syntax text, but the file is in reStructuredText (RST) format. It should be converted to RST link syntax.
| [atomic block](https://docs.djangoproject.com/en/4.2/topics/db/transactions/#controlling-transactions-explicitly). | |
| atomic block <https://docs.djangoproject.com/en/4.2/topics/db/transactions/#controlling-transactions-explicitly>__. |
|
Switching to draft until presubmits are green |
|
@ohmayr , We'll need to disable the failing tests, file a bug to follow up on it and disable releases for this package while we investigate |
|
We may be able to wait for #16624 |
The README.rst change is just using a full copy of the parent README.rst instead of an include. (That is then consistent with all other packages.)
|
I've now rebased and regenerated, marked ready for review - if it's green, we should merge. |
|
@parthea: Done, but there are still two failures. I'm rerunning them to see if they're flakes. |
|
The tests were flaky - I've filed #16839 to cover that, but I think we'd be okay to merge this now. |
4 participants
The README.rst change is just using a full copy of the parent README.rst instead of an include. (That is then consistent with all other packages.)