-
Notifications
You must be signed in to change notification settings - Fork 11
chore: migrate to main branch #111
Conversation
plamut
left a comment
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 😴 |
parthea
left a comment
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
|
|
||
| .. |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
| 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
| @@ -1 +1 @@ | |||
| See https://github.com/GoogleCloudPlatform/python-docs-samples/blob/master/AUTHORING_GUIDE.md No newline at end of file | |||
| See https://github.com/GoogleCloudPlatform/python-docs-samples/blob/main/AUTHORING_GUIDE.md No newline at end of file | |||
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 🦕