Skip to content

Conversation

@rdblue
Copy link
Contributor

@rdblue rdblue commented Apr 3, 2022

This fixes a couple of minor comments from #4416 and refactors to use common decimal and datetime util functions.

@rdblue rdblue requested a review from danielcweeks April 3, 2022 18:50
@github-actions github-actions bot added the python label Apr 3, 2022
@rdblue rdblue force-pushed the python-minor-refactor branch 2 times, most recently from 5473911 to b16164f Compare April 3, 2022 19:01
@rdblue rdblue requested a review from samredai April 3, 2022 19:02
@rdblue rdblue force-pushed the python-minor-refactor branch 2 times, most recently from c4ed6df to 4787d24 Compare April 3, 2022 19:11
@jun-he
Copy link
Collaborator

jun-he commented Apr 4, 2022

LGTM, thanks @rdblue

Copy link
Contributor

@samredai samredai left a comment

Choose a reason for hiding this comment

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

One suggestion but otherwise LGTM

Copy link
Contributor

@danielcweeks danielcweeks left a comment

Choose a reason for hiding this comment

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

Minor question on funciton name (non-blocker), but looks good to me.

@rdblue rdblue merged commit 48c24c6 into apache:master Apr 4, 2022
@rdblue
Copy link
Contributor Author

rdblue commented Apr 4, 2022

Thanks for reviewing, @jun-he, @samredai, and @danielcweeks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants