Skip to content

Conversation

@haiyuan-eng-google
Copy link
Contributor

No description provided.

Copy link
Collaborator

@koverholt koverholt left a comment

Choose a reason for hiding this comment

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

The PR title indicates a move to the plugins dir, but the doc is actually being moved to the observability directory. Do you want to move this to the plugins dir instead?

Also:

@haiyuan-eng-google haiyuan-eng-google changed the title Move BigQuery Agent Analytics docs to plugins directory and update the plugin doc to v2 Move BigQuery Agent Analytics docs to observability directory and update the plugin doc to v2 Dec 12, 2025
@haiyuan-eng-google
Copy link
Contributor Author

The PR title indicates a move to the plugins dir, but the doc is actually being moved to the observability directory. Do you want to move this to the plugins dir instead?

Also:

Thanks for the review. @koverholt, actually this PR is aiming to moving the plugin into observability directory. Update the title also update the mkdocs.yml. Please take another look, thanks

Copy link
Collaborator

@koverholt koverholt left a comment

Choose a reason for hiding this comment

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

Thanks for the fixes. I didn't realize that there was a rewrite of the page since GitHub is not showing the diffs due to the file being moved. So we'll need to do a more thorough review of the content than I initially thought.

The biggest issue that I see is whether we want to include the full documentation on the older deprecated version in the second half of the page. Other than that, there are some rendering issues like the list in the "IAM Permissions" section is not rendering correctly and other consistency issues.

I left comments for these, and will also cc @joefernandez for review as well due to the scope of the changes.

<br>
<hr>

## Deprecated (v < 1.21.0)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think we want to include the full content of the old / deprecated version, but I will defer to @joefernandez on this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @koverholt , @joefernandez please provide guidance on this, thanks

Copy link
Collaborator

Choose a reason for hiding this comment

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

@haiyuan-eng-google DO NOT preserve multiple versions of content in a single document; this practice is confusing to you users. Remove the deprecated content from this page. If you want to preserve it, consider adding it as a readme file that's kept with the source code and labeled appropriately, and if you want, link to that deprecated content from here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed the deprecated contents, thanks

@joefernandez joefernandez dismissed koverholt’s stale review December 12, 2025 23:54

Change request resolved: removed deprecated content

@joefernandez joefernandez merged commit dd8d30d into google:main Dec 12, 2025
5 checks passed
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.

3 participants