Skip to content

[docs] Add doc for hive.copy-on-first-write-configuration-enabled#25443

Merged
imjalpreet merged 1 commit intoprestodb:masterfrom
nishithakbhaskaran:doc-changes
Jul 16, 2025
Merged

[docs] Add doc for hive.copy-on-first-write-configuration-enabled#25443
imjalpreet merged 1 commit intoprestodb:masterfrom
nishithakbhaskaran:doc-changes

Conversation

@nishithakbhaskaran
Copy link
Contributor

@nishithakbhaskaran nishithakbhaskaran commented Jun 26, 2025

Description

Add doc for hive.copy-on-first-write-configuration-enabled in presto-docs/src/main/sphinx/connector/hive.rst

Motivation and Context

#25425
#25420

Impact

Test Plan

Contributor checklist

  • Please make sure your submission complies with our contributing guide, in particular code style and commit standards.
  • PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced.
  • Documented new properties (with its default value), SQL syntax, functions, or other functionality.
  • If release notes are required, they follow the release notes guidelines.
  • Adequate tests were added if applicable.
  • CI passed.

Release Notes

Please follow release notes guidelines and fill in the release notes below.

== RELEASE NOTES ==

Documentation Changes
* Add doc for hive.copy-on-first-write-configuration-enabled in  presto-docs/src/main/sphinx/connector/hive.rst

@prestodb-ci prestodb-ci added the from:IBM PR from IBM label Jun 26, 2025
@github-actions github-actions bot added the docs label Jun 26, 2025
@github-project-automation github-project-automation bot moved this to 🆕 Unprioritized in Presto Documentation Jun 26, 2025
imjalpreet
imjalpreet requested changes Jun 26, 2025
View reviewed changes
Copy link
Member

@imjalpreet imjalpreet left a comment

Choose a reason for hiding this comment

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

@nishithakbhaskaran, thank you. I have reviewed from the technical side, and I think we might need to make some formatting changes for the table. @steveburnett can confirm on that.

@github-project-automation github-project-automation bot moved this from 🆕 Unprioritized to 🏗 In progress in Presto Documentation Jun 26, 2025
@nishithakbhaskaran nishithakbhaskaran force-pushed the doc-changes branch 2 times, most recently from 5a49b05 to 9b8c86c Compare June 26, 2025 11:08
@nishithakbhaskaran nishithakbhaskaran marked this pull request as ready for review June 26, 2025 11:08
@prestodb-ci prestodb-ci requested review from a team, Mariamalmesfer and pratyakshsharma and removed request for a team June 26, 2025 11:08
steveburnett
steveburnett requested changes Jun 26, 2025
View reviewed changes
Copy link
Contributor

@steveburnett steveburnett left a comment

Choose a reason for hiding this comment

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

Thanks for the doc! A couple of formatting nits, and a suggestion of how to handle the concern of referencing to a PR or issue.

steveburnett
steveburnett previously approved these changes Jun 30, 2025
View reviewed changes
Copy link
Contributor

@steveburnett steveburnett left a comment

Choose a reason for hiding this comment

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

LGTM! (docs)

Pull updated branch, new local doc build, looks good. Thanks!

@nishithakbhaskaran nishithakbhaskaran force-pushed the doc-changes branch 2 times, most recently from abf27e4 to f244125 Compare July 16, 2025 07:27
@nishithakbhaskaran
Copy link
Contributor Author

nishithakbhaskaran commented Jul 16, 2025

@imjalpreet @steveburnett Rebased the branch with merge conflicts and the previous approval got dismissed. Could you please take a look once more?

imjalpreet
imjalpreet reviewed Jul 16, 2025
View reviewed changes
Copy link
Member

@imjalpreet imjalpreet left a comment

Choose a reason for hiding this comment

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

LGTM, a minor nit.

presto-docs/src/main/sphinx/connector/hive.rst Outdated
standard Hadoop Configuration object, extending its
behaviour by introducing an additional layer of
indirection.However,many third-party libraries that
integrate with Presto relay directly on the Configuration
Copy link
Member

@imjalpreet imjalpreet Jul 16, 2025

Choose a reason for hiding this comment

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

Suggested change
integrate with Presto relay directly on the Configuration
integrate with Presto rely directly on the Configuration

Copy link
Contributor Author

@nishithakbhaskaran nishithakbhaskaran Jul 16, 2025

Choose a reason for hiding this comment

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

Addressed.

@nishithakbhaskaran nishithakbhaskaran force-pushed the doc-changes branch 2 times, most recently from fee76bf to eb6e224 Compare July 16, 2025 13:20
Mariamalmesfer
Mariamalmesfer approved these changes Jul 16, 2025
View reviewed changes
Copy link
Contributor

@Mariamalmesfer Mariamalmesfer left a comment

Choose a reason for hiding this comment

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

LGTM

steveburnett
steveburnett requested changes Jul 16, 2025
View reviewed changes
Copy link
Contributor

@steveburnett steveburnett left a comment

Choose a reason for hiding this comment

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

One nit, looks good otherwise!

steveburnett
steveburnett approved these changes Jul 16, 2025
View reviewed changes
Copy link
Contributor

@steveburnett steveburnett left a comment

Choose a reason for hiding this comment

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

LGM! (docs)

Pull updated branch, new local doc build, looks good. Thanks!

pratyakshsharma
pratyakshsharma approved these changes Jul 16, 2025
View reviewed changes
Copy link
Contributor

@pratyakshsharma pratyakshsharma 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.

@github-project-automation github-project-automation bot moved this from 🏗 In progress to ✅ Done in Presto Documentation Jul 16, 2025
@imjalpreet imjalpreet merged commit 22d2729 into prestodb:master Jul 16, 2025
105 checks passed
@nishithakbhaskaran nishithakbhaskaran deleted the doc-changes branch July 17, 2025 05:24
This was referenced Jul 24, 2025
Merged
Merged
@prestodb-ci prestodb-ci mentioned this pull request Jul 28, 2025
Merged
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@imjalpreet imjalpreet imjalpreet approved these changes

@steveburnett steveburnett steveburnett approved these changes

@pratyakshsharma pratyakshsharma pratyakshsharma approved these changes

@elharo elharo Awaiting requested review from elharo elharo is a code owner

+1 more reviewer

@Mariamalmesfer Mariamalmesfer Mariamalmesfer approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

docs from:IBM PR from IBM

Projects

Presto Documentation
Status: ✅ Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@nishithakbhaskaran @imjalpreet @steveburnett @pratyakshsharma @Mariamalmesfer @prestodb-ci