Update to account for XContent refactor in 2.x by MaxKsyunz · Pull Request #1485 · opensearch-project/sql · GitHub
Skip to content

Update to account for XContent refactor in 2.x#1485

Merged
Yury-Fridlyand merged 3 commits into
opensearch-project:2.xfrom
Bit-Quill:2.x-build-fixes
Mar 29, 2023
Merged

Update to account for XContent refactor in 2.x#1485
Yury-Fridlyand merged 3 commits into
opensearch-project:2.xfrom
Bit-Quill:2.x-build-fixes

Conversation

@MaxKsyunz

Copy link
Copy Markdown
Collaborator

Description

Cherry pick of cd95bc5 plus fixes for 2.x-only changes.

Issues Resolved

#1475

Check List

  • All tests pass, including unit test, integration test and doctest
  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Max Ksyunz added 2 commits March 29, 2023 09:36
Xcontent classes were moved in this PR opensearch-project/OpenSearch#5902 causing builds to fail.

---------

Signed-off-by: MaxKsyunz <maxk@bitquilltech.com>
(cherry picked from commit cd95bc5)
Signed-off-by: MaxKsyunz <maxk@bitquilltech.com>
@MaxKsyunz MaxKsyunz requested a review from a team as a code owner March 29, 2023 17:21
@MaxKsyunz MaxKsyunz changed the title Update to account for XContent Refactor in 2.x Update to account for XContent refactor in 2.x Mar 29, 2023
@codecov-commenter

codecov-commenter commented Mar 29, 2023

Copy link
Copy Markdown

Yury-Fridlyand
Yury-Fridlyand previously approved these changes Mar 29, 2023

@Yury-Fridlyand Yury-Fridlyand left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Import reorder may be needed to satisfy the checkstyle.

rupal-bq
rupal-bq previously approved these changes Mar 29, 2023
Comment thread build.gradle Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why was this added?

@MaxKsyunz MaxKsyunz Mar 29, 2023

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Good catch! I'll remove it -- it does not apply to 2.x branch.
It was added when main was bumped to 3.0.

The PR to main moved that line because the build was picking up dependencies from that random repository instead of sonatype and failing.
image

derek-ho
derek-ho previously approved these changes Mar 29, 2023

@derek-ho derek-ho left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

mostly LGTM

Signed-off-by: MaxKsyunz <maxk@bitquilltech.com>
@MaxKsyunz MaxKsyunz dismissed stale reviews from derek-ho, rupal-bq, and Yury-Fridlyand via 7d3793b March 29, 2023 19:49

@dai-chen dai-chen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for the fix!

@Yury-Fridlyand Yury-Fridlyand merged commit 56eea47 into opensearch-project:2.x Mar 29, 2023
@Yury-Fridlyand Yury-Fridlyand deleted the 2.x-build-fixes branch March 29, 2023 20:52
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.

6 participants