-
Notifications
You must be signed in to change notification settings - Fork 13
chore: migrate to main branch #111
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.
LGTM, could not find any references to master
, whitelist
, etc. anymore apart from those in replacement rules.
BTW, when can we expect the code generator to generate the code using the main
branch so that those replacement rules will not be needed anymore?
@plamut we'll sweep through the repositories first, then make template updates, then revert the replacement rules back. It shouldn't be long! |
OK then, although it feels like extra work. :) |
It is, but I think we're doing it so that we don't accidentally move stuff to the main branch and have things crash (template change sends 120+ PRs simultaneously), doing this slowly will ensure we don't break things along the way and minimize the impact as little as possible to prod, at the expense of some manhours which I think is affordable... maybe 😴 |
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.
Minor observations, otherwise LGTM.
README.rst
Outdated
@@ -10,7 +10,7 @@ creation of service accounts, which you can use to authenticate to Google and ma | |||
- `Product Documentation`_ | |||
|
|||
.. |ga| image:: https://img.shields.io/badge/support-GA-gold.svg | |||
:target: https://github.com/googleapis/google-cloud-python/blob/master/README.rst#general-availability | |||
:target: https://github.com/googleapis/google-cloud-python/blob/main/README.rst#general-availability |
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 get 404 with the new link
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.
Presumably this will resolve itself once that repo is migrated and the main
branch is created?
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.
It will, but to Tony's point we shouldn't let users see a 404. I'll have it reverted for now.
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.
Ah, that's true, I thought the migration will happen simultaneously, but that's somewhat optimistic. 🙂 Tony is right here.
UPGRADING.md
Outdated
@@ -72,7 +72,7 @@ In `google-cloud-iam<2.0.0`, parameters required by the API were positional para | |||
|
|||
In the 2.0.0 release, all methods have a single positional parameter `request`. Method docstrings indicate whether a parameter is required or optional. | |||
|
|||
Some methods have additional keyword only parameters. The available parameters depend on the [`google.api.method_signature` annotation](https://github.com/googleapis/googleapis/blob/master/google/iam/credentials/v1/iamcredentials.proto#L49) specified by the API producer. | |||
Some methods have additional keyword only parameters. The available parameters depend on the [`google.api.method_signature` annotation](https://github.com/googleapis/googleapis/blob/main/google/iam/credentials/v1/iamcredentials.proto#L49) specified by the API producer. |
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 get 404 with the new link
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.
samples/AUTHORING_GUIDE.md
Outdated
See https://github.com/GoogleCloudPlatform/python-docs-samples/blob/main/AUTHORING_GUIDE.md |
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 get 404 with the new link
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.
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes #110 🦕