Skip to content

[CALCITE-5782] Add TO_HEX and FROM_HEX functions (enabled in BigQuery library)#3290

Merged
ILuffZhe merged 1 commit intoapache:mainfrom
zoudan:CALCITE-5782
Jul 10, 2023
Merged

[CALCITE-5782] Add TO_HEX and FROM_HEX functions (enabled in BigQuery library)#3290
ILuffZhe merged 1 commit intoapache:mainfrom
zoudan:CALCITE-5782

Conversation

@zoudan
Copy link
Copy Markdown
Contributor

@zoudan zoudan commented Jul 3, 2023

No description provided.

Copy link
Copy Markdown
Contributor

@tanclary tanclary 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 small question

Copy link
Copy Markdown
Contributor

@ILuffZhe ILuffZhe left a comment

Choose a reason for hiding this comment

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

Overall looks good! Left some minor comments.

@ILuffZhe ILuffZhe self-assigned this Jul 6, 2023
Copy link
Copy Markdown
Contributor

@ILuffZhe ILuffZhe left a comment

Choose a reason for hiding this comment

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

+1.
Hi @tanclary , do you have any other comments? I think this PR is in a good shape now.

@tanclary
Copy link
Copy Markdown
Contributor

tanclary commented Jul 6, 2023

+1. Hi @tanclary , do you have any other comments? I think this PR is in a good shape now.

I think it looks good as well, the only thing I am confused about is the documentation question but I ask more out of curiosity.

@ILuffZhe ILuffZhe added the LGTM-will-merge-soon Overall PR looks OK. Only minor things left. label Jul 8, 2023
@ILuffZhe
Copy link
Copy Markdown
Contributor

ILuffZhe commented Jul 8, 2023

Hi @zoudan, could you please rebase and squash the commits? I think the PR is ready to merge.

@sonarqubecloud
Copy link
Copy Markdown

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

89.3% 89.3% Coverage
0.0% 0.0% Duplication

@ILuffZhe ILuffZhe merged commit 8d33c2e into apache:main Jul 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

LGTM-will-merge-soon Overall PR looks OK. Only minor things left.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants