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

Add support for passing time.Duration #124

Merged
merged 1 commit into from
Sep 19, 2024

Conversation

joris-bright
Copy link
Contributor

No description provided.

Copy link

cla-bot bot commented Sep 12, 2024

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

Copy link

cla-bot bot commented Sep 12, 2024

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

Copy link
Member

@nineinchnick nineinchnick left a comment

Choose a reason for hiding this comment

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

The only thing we need to change is avoid precision loss. Other comments are nitpicks. Thanks for working on this!

README.md Outdated Show resolved Hide resolved
trino/serial.go Outdated Show resolved Hide resolved
trino/serial.go Outdated Show resolved Hide resolved
trino/serial.go Outdated Show resolved Hide resolved
trino/serial.go Outdated Show resolved Hide resolved
trino/serial.go Outdated Show resolved Hide resolved
trino/serial.go Outdated Show resolved Hide resolved
trino/serial.go Outdated Show resolved Hide resolved
trino/serial.go Outdated Show resolved Hide resolved
trino/serial.go Outdated Show resolved Hide resolved
Copy link

cla-bot bot commented Sep 17, 2024

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

2 similar comments
Copy link

cla-bot bot commented Sep 17, 2024

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

Copy link

cla-bot bot commented Sep 17, 2024

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

@joris-bright
Copy link
Contributor Author

Let me know if there are any more changes you'd like to see. (I have send a mail for cla when I opened the pr. Do I still need to do something or does it just take some time?)

README.md Outdated Show resolved Hide resolved
trino/integration_test.go Show resolved Hide resolved
@nineinchnick
Copy link
Member

Let me know if there are any more changes you'd like to see.

Just a nitpick and that integration test is weird.

I have send a mail for cla when I opened the pr. Do I still need to do something or does it just take some time?

The CLAs get processed every two weeks or so. I'm watching that and will merge this PR when it gets cleared. Thanks for working on it!

@nineinchnick
Copy link
Member

One last thing - please squash commits and remove the prefix from the commit message. You can do that after we resolve all review comments. Force-pushing one updated commit is also fine, GitHub shows the diff anyway.

Copy link

cla-bot bot commented Sep 17, 2024

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

Copy link

cla-bot bot commented Sep 17, 2024

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

@joris-bright joris-bright changed the title feat(serial): add support for passing time.Duration Add support for passing time.Duration Sep 17, 2024
Copy link

cla-bot bot commented Sep 17, 2024

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

@nineinchnick
Copy link
Member

@cla-bot check

@cla-bot cla-bot bot added the cla-signed label Sep 19, 2024
Copy link

cla-bot bot commented Sep 19, 2024

The cla-bot has been summoned, and re-checked this pull request!

@nineinchnick nineinchnick merged commit 2b16344 into trinodb:master Sep 19, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants