Skip to content

Docs/cloudrun vpc improve#3100

Open
NaniToka wants to merge 2 commits intogoogleapis:mainfrom
NaniToka:docs/cloudrun-vpc-improve
Open

Docs/cloudrun vpc improve#3100
NaniToka wants to merge 2 commits intogoogleapis:mainfrom
NaniToka:docs/cloudrun-vpc-improve

Conversation

@NaniToka
Copy link
Copy Markdown
Contributor

Description

Should include a concise description of the changes (bug or feature), it's
impact, along with a summary of the solution

PR Checklist

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:

  • Make sure you reviewed
    CONTRIBUTING.md
  • Make sure to open an issue as a
    bug/issue
    before writing your code! That way we can discuss the change, evaluate
    designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)
  • Make sure to add ! if this involve a breaking change

🛠️ Fixes #<issue_number_goes_here>

@NaniToka NaniToka requested a review from a team as a code owner April 19, 2026 00:18
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the .gitignore file to exclude .DS_Store and performs a comprehensive rewrite of the UPGRADING.md guide to reflect changes in version 1.0.0, including repository renaming, endpoint deprecations, and configuration schema updates. The review feedback identifies several technical inaccuracies in the documentation that need correction: restoring the link to the MCP tool naming specification, clarifying that certain CLI flags are currently deprecated rather than fully removed, and ensuring the configuration field name matches the implementation (authServices instead of authService).

Comment thread UPGRADING.md

Invalid names will fail MCP initialization.

Reference: MCP specification for tool naming.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The link to the MCP specification for tool naming was removed. Restoring it (or updating it to the official documentation) provides valuable context for users regarding the strict naming requirements, specifically that tool names must be in snake_case.

Suggested change
Reference: MCP specification for tool naming.
Reference: [MCP specification for tool naming](https://modelcontextprotocol.io/specification/basic/tools#name-requirements).
References
  1. Tool names must be in snake_case.

Comment thread UPGRADING.md

### 4. CLI Flag Changes

Removed:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

These flags are currently marked as deprecated in the code (cmd/internal/flags.go), not removed. They still function but emit a warning. Describing them as 'Deprecated' is more accurate and prevents users from thinking their existing automation will break immediately.

Suggested change
Removed:
Deprecated (to be removed in a future release):

Comment thread UPGRADING.md

### 6. Configuration Field Rename

* `authSources` → `authService`
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The nested configuration format uses the key authServices (plural), as seen in cmd/internal/config.go and the Config struct. The migration logic also specifically looks for authServices. This should be updated to match the code and avoid confusion for users migrating from the nested format.

Suggested change
* `authSources``authService`
* authServiceauthServices

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants