Skip to content

Created s3select package for presto-hive#18730

Merged
pettyjamesm merged 1 commit intoprestodb:masterfrom
dnanuti:s3select-package-refactoring
Dec 5, 2022
Merged

Created s3select package for presto-hive#18730
pettyjamesm merged 1 commit intoprestodb:masterfrom
dnanuti:s3select-package-refactoring

Conversation

@dnanuti
Copy link

@dnanuti dnanuti commented Nov 29, 2022

Refactored S3Select related classes in own package

Test plan - These changes do not bring additional behavior. Existing tests are covering this. Ran existing unit tests.

Please make sure your submission complies with our Development, Formatting, and Commit Message guidelines. Don't forget to follow our attribution guidelines for any code copied from other projects.

Fill in the release notes towards the bottom of the PR description.
See Release Notes Guidelines for details.

== NO RELEASE NOTE ==

@dnanuti dnanuti requested a review from a team as a code owner November 29, 2022 17:54
@dnanuti dnanuti requested a review from presto-oss November 29, 2022 17:54
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Nov 29, 2022

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: dnanuti / name: Diana Nanuti (8ee044d21dcb4c1fecb233c5bd60ab4876787ee7)

@dnanuti
Copy link
Author

dnanuti commented Dec 1, 2022

@pettyjamesm would you please review?

@ajaygeorge
Copy link
Contributor

@dnanuti Please clean up the commit message to rewrite it in imperative form.
See https://cbea.ms/git-commit/

Since this doesn't require a release note you can remove the Release note section as well.

Copy link
Contributor

@ajaygeorge ajaygeorge left a comment

Choose a reason for hiding this comment

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

LGTM % commit message

Copy link
Contributor

@pettyjamesm pettyjamesm left a comment

Choose a reason for hiding this comment

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

+1 for rewording the commit title and message to imperative tone, otherwise LGTM.

Refactored S3Select related classes in their own package.
@dnanuti dnanuti force-pushed the s3select-package-refactoring branch from 8ee044d to 425ddf3 Compare December 5, 2022 17:00
@dnanuti
Copy link
Author

dnanuti commented Dec 5, 2022

LGTM % commit message

Updated, thanks!

@pettyjamesm pettyjamesm merged commit 9b74150 into prestodb:master Dec 5, 2022
@pettyjamesm
Copy link
Contributor

Merged, thanks!

@dnanuti dnanuti deleted the s3select-package-refactoring branch December 6, 2022 22:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants