Skip to content

Conversation

@liuyongvs
Copy link
Contributor

@liuyongvs liuyongvs commented Aug 11, 2022

What is the purpose of the change

Add built-in conv function.

@flinkbot
Copy link
Collaborator

flinkbot commented Aug 11, 2022

CI report:

Bot commands The @flinkbot bot supports the following commands:
  • @flinkbot run azure re-run the last Azure build

@liuyongvs liuyongvs force-pushed the add_conv_function branch 3 times, most recently from eba4c9d to 5cf2143 Compare August 15, 2022 03:58
@liuyongvs
Copy link
Contributor Author

hi @twalthr @wuchong do you have time to review it?

return null;
}
byte[] bytes =
BaseConversionUtils.conv(
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please clarify why BaseConversionUtils is required and e.g. java.lang.Long#toString(long, int) is not sutable here?

null,
STRING().nullable()))

// fromBase cannot be negative.
Copy link
Contributor

Choose a reason for hiding this comment

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

it is also mentioned that min base is 2 and max is 36
it would be great to see tests for such cases when base is 1 or 37 e.g.

@snuyanzin
Copy link
Contributor

Thanks for the contribution
It would be great to have a filled template for PR
also I left some comments

@liuyongvs liuyongvs closed this Sep 5, 2022
@liuyongvs liuyongvs deleted the add_conv_function branch September 5, 2022 05:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants