Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(api): remove IntegerValue.label #10595

Merged
merged 1 commit into from
Dec 19, 2024

Conversation

cpcloud
Copy link
Member

@cpcloud cpcloud commented Dec 18, 2024

Closes #9612.

BREAKING CHANGE: IntegerValue.label is redundant with the IntegerValue.cases method, use that instead. Replace expr.label(labels) with expr.cases(*enumerate(labels))

@cpcloud cpcloud added this to the 10.0 milestone Dec 18, 2024
@github-actions github-actions bot added tests Issues or PRs related to tests impala The Apache Impala backend postgres The PostgreSQL backend risingwave The RisingWave backend labels Dec 18, 2024
@cpcloud cpcloud force-pushed the remove-labels-method branch from 0e1d154 to bc7acbd Compare December 18, 2024 13:09
Copy link
Member

@gforsyth gforsyth left a comment

Choose a reason for hiding this comment

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

Looks good to me -- maybe we can add an example of the new usage to the breaking message?

@NickCrews
Copy link
Contributor

NickCrews commented Dec 19, 2024

+1 to including a guide/example on how to migrate in the commit message, which can then get included in the changelog when that gets authored.

I would also not categorize this as a refactor(API) commit if there is a tag for breaking(API) or something. Or we should add one. refactor as a tag is definitely misleading.

Besides those two things looks great! Thanks.

@cpcloud
Copy link
Member Author

cpcloud commented Dec 19, 2024

The category of the change and whether it's breaking are two mostly independent properties.

@cpcloud cpcloud force-pushed the remove-labels-method branch from bc7acbd to 5d10a82 Compare December 19, 2024 10:45
BREAKING CHANGE: `IntegerValue.label` is redundant with the `IntegerValue.cases` method, use that instead. Replace `expr.label(labels)` with `expr.cases(*enumerate(labels))`
@cpcloud cpcloud force-pushed the remove-labels-method branch from 5d10a82 to 90391a8 Compare December 19, 2024 10:46
@cpcloud cpcloud enabled auto-merge (rebase) December 19, 2024 10:51
@cpcloud cpcloud merged commit 6278f7e into ibis-project:main Dec 19, 2024
87 checks passed
@cpcloud cpcloud deleted the remove-labels-method branch December 19, 2024 12:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
impala The Apache Impala backend postgres The PostgreSQL backend risingwave The RisingWave backend tests Issues or PRs related to tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: remove IntegerValue.label()
3 participants