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

Documented human_readable_seconds and added function to list.rst #7835

Closed
wants to merge 1 commit into from

Conversation

cwise827
Copy link
Contributor

@cwise827 cwise827 commented May 5, 2021

@cla-bot
Copy link

cla-bot bot commented May 5, 2021

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please submit the signed CLA to [email protected]. For more information, see https://github.com/trinodb/cla.

@martint martint requested a review from mosabua May 5, 2021 17:31
@rosewms rosewms requested review from m57lyra, Ordinant and rosewms May 5, 2021 18:39
@mosabua mosabua added the docs label May 5, 2021
Copy link
Member

@mosabua mosabua left a comment

Choose a reason for hiding this comment

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

Looks good. A bit of tweaking and CLA required.

@@ -309,14 +309,18 @@ Unit Description

.. function:: human_readable_seconds(double) -> varchar

Returns ``seconds`` expressed in terms of ``human readable interval``::
Parses value of ``seconds`` into a human readable varchar containing
Copy link
Member

Choose a reason for hiding this comment

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

Maybe

Parses the double value of seconds into a human readable string containing ...

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed

Copy link
Member

Choose a reason for hiding this comment

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

Instead of "parses", I would use "formats".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggestions have been implemented @mosabua. I emailed a signed CLA to [email protected] on Tuesday at 12:47 AM EDT. Is there anything else I need to do for the CLA?

Trino Screenshot 2

@cla-bot
Copy link

cla-bot bot commented May 6, 2021

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please submit the signed CLA to [email protected]. For more information, see https://github.com/trinodb/cla.

Copy link
Member

@mosabua mosabua left a comment

Choose a reason for hiding this comment

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

This looks good now. Please just rebase the commit so this PR has only one commit.

@cla-bot
Copy link

cla-bot bot commented May 8, 2021

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please submit the signed CLA to [email protected]. For more information, see https://github.com/trinodb/cla.

@cla-bot
Copy link

cla-bot bot commented May 8, 2021

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

@martint
Copy link
Member

martint commented May 12, 2021

Merged as 735ecd2, thanks!

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.

4 participants