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

Skip geojson test for version below 2.4 #4860

Merged
merged 3 commits into from
Oct 22, 2022

Conversation

heemin32
Copy link
Contributor

@heemin32 heemin32 commented Oct 20, 2022

Signed-off-by: Heemin Kim [email protected]

Description

Skip geojson test for version below 2.4

Issues Resolved

#4852

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed per the DCO using --signoff
  • Commit changes are listed out in CHANGELOG.md file (See: Changelog)

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.

@heemin32 heemin32 requested review from a team and reta as code owners October 20, 2022 20:05
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@codecov-commenter
Copy link

codecov-commenter commented Oct 20, 2022

Codecov Report

Merging #4860 (14acda2) into main (d85f8cd) will decrease coverage by 1.03%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##               main    #4860      +/-   ##
============================================
- Coverage     71.79%   70.75%   -1.04%     
+ Complexity    62907    57863    -5044     
============================================
  Files          4689     4689              
  Lines        300505   276910   -23595     
  Branches      46129    40300    -5829     
============================================
- Hits         215750   195937   -19813     
+ Misses        67829    64667    -3162     
+ Partials      16926    16306     -620     
Impacted Files Coverage Δ
...luster/routing/allocation/RoutingExplanations.java 41.37% <0.00%> (-58.63%) ⬇️
.../java/org/opensearch/node/NodeClosedException.java 50.00% <0.00%> (-50.00%) ⬇️
...ava/org/opensearch/action/NoSuchNodeException.java 0.00% <0.00%> (-50.00%) ⬇️
...a/org/opensearch/tasks/TaskCancelledException.java 50.00% <0.00%> (-50.00%) ⬇️
.../org/opensearch/http/HttpReadTimeoutException.java 50.00% <0.00%> (-50.00%) ⬇️
.../admin/cluster/reroute/ClusterRerouteResponse.java 55.00% <0.00%> (-45.00%) ⬇️
...adcast/BroadcastShardOperationFailedException.java 55.55% <0.00%> (-44.45%) ⬇️
...h/action/ingest/SimulateDocumentVerboseResult.java 60.71% <0.00%> (-39.29%) ⬇️
...java/org/opensearch/threadpool/ThreadPoolInfo.java 56.25% <0.00%> (-37.50%) ⬇️
...n/admin/indices/readonly/AddIndexBlockRequest.java 35.71% <0.00%> (-35.72%) ⬇️
... and 665 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@heemin32
Copy link
Contributor Author

heemin32 commented Oct 20, 2022

#4842 need to be merged to make the test pass.

@andrross
Copy link
Member

The general best practice here, I think, is that on main you should put the skip to " - 2.99.99" since this feature only exists in 3.0 and newer for the moment. Then you can backport that commit, and in the backport PR you can change the skip value to " - 2.3.99" (or whatever is just below the next minor version on the backport branch). Then after the backport is committed, you can submit a new change to main to change the skip value to " - 2.3.99". I think that's the only way to do it without breaking things along the way.

@heemin32
Copy link
Contributor Author

The general best practice here, I think, is that on main you should put the skip to " - 2.99.99" since this feature only exists in 3.0 and newer for the moment. Then you can backport that commit, and in the backport PR you can change the skip value to " - 2.3.99" (or whatever is just below the next minor version on the backport branch). Then after the backport is committed, you can submit a new change to main to change the skip value to " - 2.3.99". I think that's the only way to do it without breaking things along the way.

Is it allowed to raise PR for 2.x first and another for main later? This will reduce the number of code changes and PR.

@andrross
Copy link
Member

Is it allowed to raise PR for 2.x first and another for main later?

Per our defined branching strategy, all merges should happen on main and be backported to the other branches.

@andrross
Copy link
Member

@heemin32 #4842 has been merged, can you rebase this one to fix the changelog conflicts?

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@andrross
Copy link
Member

Created issue for previous test failure: #4881

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@andrross andrross merged commit 49a9b81 into opensearch-project:main Oct 22, 2022
@heemin32 heemin32 deleted the geo_point_test branch October 22, 2022 00:57
@heemin32
Copy link
Contributor Author

@andrross Thanks for the quick response!

ashking94 pushed a commit to ashking94/OpenSearch that referenced this pull request Nov 7, 2022
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