Skip to content

Add missing config properties to Hive docs#16808

Merged
ebyhr merged 1 commit intotrinodb:masterfrom
colebow:colebow/add-missing-hive-properties
Apr 6, 2023
Merged

Add missing config properties to Hive docs#16808
ebyhr merged 1 commit intotrinodb:masterfrom
colebow:colebow/add-missing-hive-properties

Conversation

@colebow
Copy link
Copy Markdown
Member

@colebow colebow commented Mar 30, 2023

Description

Lifting some of the config docs improvements from #10831. Attribution to @findinpath is in the commit message.

Release notes

(x) This is not user-visible or docs only and no release notes are required.

@cla-bot cla-bot bot added the cla-signed label Mar 30, 2023
@colebow colebow requested review from ebyhr and findinpath March 30, 2023 17:48
@colebow colebow marked this pull request as ready for review March 30, 2023 17:48
@colebow colebow requested a review from m57lyra March 30, 2023 17:48
Copy link
Copy Markdown
Contributor

@findinpath findinpath left a comment

Choose a reason for hiding this comment

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

I verified the default values for all the newly added properties and everything seems to correspond to the Trino codebase.

There are small formatting issues, but besides that I think the PR is good to go.

Some of these properties could have a story of their own to be expanded to give more context to the users, but having their existence documented is a really good addition.

@github-actions github-actions bot added the docs label Mar 30, 2023
@colebow colebow force-pushed the colebow/add-missing-hive-properties branch from 4dbfe74 to e659161 Compare March 31, 2023 17:44
@colebow
Copy link
Copy Markdown
Member Author

colebow commented Mar 31, 2023

There are small formatting issues, but besides that I think the PR is good to go.

You're being generous with the world "small," I've got no idea what happened there. 😆

@colebow colebow requested a review from findinpath March 31, 2023 17:45
Copy link
Copy Markdown
Member

@mosabua mosabua Mar 31, 2023

Choose a reason for hiding this comment

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

A convention we chose for a lot of table is to NOT have a default column because a lot of properties dont have a default defined.. so I suggest to undo these unrelated changes.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think it makes sense, because this is adding a lot of rows to the table that do have default values.

Copy link
Copy Markdown
Contributor

@m57lyra m57lyra left a comment

Choose a reason for hiding this comment

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

Couple of things to fix

@colebow colebow force-pushed the colebow/add-missing-hive-properties branch from e659161 to 4a246b0 Compare April 4, 2023 18:46
@findinpath
Copy link
Copy Markdown
Contributor

@ebyhr CPTAL over the addition of the docs for these Hive settings?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Co-authored by ...

The right syntax is Co-Authored-By (2nd - is missing).

@ebyhr ebyhr force-pushed the colebow/add-missing-hive-properties branch from 4a246b0 to 9e3faa2 Compare April 5, 2023 23:04
Co-Authored-By: Marius Grama <findinpath@gmail.com>
@ebyhr ebyhr force-pushed the colebow/add-missing-hive-properties branch from 9e3faa2 to 62ff474 Compare April 5, 2023 23:06
@ebyhr ebyhr merged commit 9ba8404 into trinodb:master Apr 6, 2023
@github-actions github-actions bot added this to the 413 milestone Apr 6, 2023
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.

5 participants