Skip to content

[clickhouse-admin-types] reorganize per RFD 619#9523

Merged
sunshowers merged 4 commits intomainfrom
sunshowers/spr/clickhouse-admin-types-reorganize-per-rfd-619
Dec 18, 2025
Merged

[clickhouse-admin-types] reorganize per RFD 619#9523
sunshowers merged 4 commits intomainfrom
sunshowers/spr/clickhouse-admin-types-reorganize-per-rfd-619

Conversation

@sunshowers
Copy link
Contributor

No description provided.

Created using spr 1.3.6-beta.1
Created using spr 1.3.6-beta.1
Created using spr 1.3.6-beta.1
Copy link
Contributor

@karencfv karencfv left a comment

Choose a reason for hiding this comment

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

Just left a note about a module, but apart from that looks good. Thanks for the last minute push to get all of these in!

Created using spr 1.3.6-beta.1
Copy link
Contributor

@karencfv karencfv left a comment

Choose a reason for hiding this comment

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

Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

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

This is more of a question for my own understanding. Why is there a separate impls module instead of having the impls live with their type in each version module?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is so that if you change a type, the implementations get carried forward to the new version without you having to copy code around. (In some cases you might need some of these methods on prior versions, but we hope/expect that that is uncommon -- and in that case, you can copy those methods and put them next to the prior version.)

@sunshowers sunshowers enabled auto-merge (squash) December 17, 2025 23:27
@sunshowers sunshowers merged commit 06c0808 into main Dec 18, 2025
18 checks passed
@sunshowers sunshowers deleted the sunshowers/spr/clickhouse-admin-types-reorganize-per-rfd-619 branch December 18, 2025 08:12
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