velox icon indicating copy to clipboard operation
velox copied to clipboard

Published 20 hours ago • verified publisher icon facebookincubator

Create utility file for encoding

Open Joe-Abraham opened this issue 1 year ago • 10 comments

Add a new utility file that shares the common logic for decoding and encoding based on the base variants.

  • Fixes https://github.com/facebookincubator/velox/issues/7294
  • Dependency - https://github.com/facebookincubator/velox/pull/10371
  • New presto functions to_base32 and from_base32( https://github.com/facebookincubator/velox/pull/8652 and https://github.com/facebookincubator/velox/pull/7672) has been raised as subsequent PRs.

Joe-Abraham avatar Feb 02 '24 06:02 Joe-Abraham

Deploy Preview for meta-velox canceled.

Name Link
Latest commit abad3806a37541d72013dfc9e18d4d287008bc33
Latest deploy log https://app.netlify.com/sites/meta-velox/deploys/6642ee55f1555c00087ffed5

netlify[bot] avatar Feb 02 '24 06:02 netlify[bot]

@aditi-pandit, I have added the test cases as you suggested. Please have a look.

Joe-Abraham avatar Mar 11 '24 09:03 Joe-Abraham

@Joe-Abraham : I've been staring at this code for a while.... So in Velox we prefer to use for loops instead of while. Please can you go over the code and rewrite to use for loops instead. Also you should try to avoid conditions within the for loop if possible.

aditi-pandit avatar Apr 11 '24 21:04 aditi-pandit

@aditi-pandit updated the code as per comments.

Joe-Abraham avatar Apr 15 '24 06:04 Joe-Abraham

@aditi-pandit can you please have a look at the changes?

Joe-Abraham avatar May 02 '24 05:05 Joe-Abraham

@bikramSingh91 Can you please have a look into this PR?

Joe-Abraham avatar May 06 '24 07:05 Joe-Abraham