Skip to content

Conversation

@feywind
Copy link
Contributor

@feywind feywind commented Dec 11, 2025

This is a follow-up for these two PRs:

#1739
#1740

This takes care of some missing pieces that were discovered by further discussion:

  • GcRuleBuilder is a more TypeScript-heavy way to validate ModifyColumnFamilies garbage collection rules, with type checking taking care of a lot of the constraints, and some minimal runtime checking for JavaScript users.
  • Rather than raw GAPIC, the focus has shifted to selective GAPIC subclasses that can augment them. This is specifically done to assist with RestoreTable and waiting for table consistency.
  • Data plane samples have been removed, as we don't want to encourage users to use them.
  • Generated samples and tests have been auto-edited to use the selective GAPIC classes.
  • Manual sample snippets have been updated to reflect the expected admin access and GcRuleBuilder usage.

There is still a CI issue that will need to be corrected before this can be merged. I don't think it's the result of this change, but I also don't want to chance it.

@feywind feywind self-assigned this Dec 11, 2025
@feywind feywind requested review from a team as code owners December 11, 2025 21:57
@feywind feywind added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Dec 11, 2025
@snippet-bot
Copy link

snippet-bot bot commented Dec 11, 2025

Here is the summary of changes.

You are about to add 3 region tags.
You are about to delete 13 region tags.

This comment is generated by snippet-bot.
If you find problems with this result, please file an issue at:
https://github.com/googleapis/repo-automation-bots/issues.
To update this comment, add snippet-bot:force-run label or use the checkbox below:

  • Refresh this comment

@product-auto-label product-auto-label bot added the size: xl Pull request size is extra large. label Dec 11, 2025
@generated-files-bot
Copy link

Warning: This pull request is touching the following templated files:

@product-auto-label product-auto-label bot added the api: bigtable Issues related to the googleapis/nodejs-bigtable API. label Dec 11, 2025
@feywind feywind changed the title Modernization part 2 feat: modernization part 2 Dec 11, 2025
@feywind feywind added owl-bot-copy owlbot:run Add this label to trigger the Owlbot post processor. labels Dec 11, 2025
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Dec 11, 2025
@product-auto-label product-auto-label bot added size: xl Pull request size is extra large. and removed size: l Pull request size is large. labels Jan 23, 2026
@feywind feywind added the owlbot:run Add this label to trigger the Owlbot post processor. label Jan 26, 2026
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Jan 26, 2026
@feywind feywind added the owlbot:run Add this label to trigger the Owlbot post processor. label Jan 26, 2026
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Jan 26, 2026
import {describe, it} from 'mocha';

describe('GcRuleBuilder', () => {
it('has working TypeScript types', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

With the new changes, are there tests anywhere that ensure the full sample from the doc https://docs.google.com/document/d/14aOXOpEbwpYJe7-p6E-PUgoy0vn4AKI7zP0Man0prLo/edit?tab=t.0#heading=h.d7jxtor70ekh runs without errors?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm pretty sure there's a very similar one in the table admin tests, but maybe we could update that one to do what's in the doc, since it's more comprehensive?

Copy link
Contributor

Choose a reason for hiding this comment

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

That sounds fine to me

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm going to go ahead and plan to do this in a different PR, maybe we can also add the family.js stuff suggested above.

Copy link
Contributor

@danieljbruce danieljbruce left a comment

Choose a reason for hiding this comment

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

LGTM with a few nits and checks.

Copy link
Contributor

@danieljbruce danieljbruce left a comment

Choose a reason for hiding this comment

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

Left a few more responses.

@feywind feywind merged commit a0f216e into main Jan 28, 2026
24 checks passed
@feywind feywind deleted the modernization-2 branch January 28, 2026 23:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: bigtable Issues related to the googleapis/nodejs-bigtable API. size: xl Pull request size is extra large.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants