-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
[Backport 2.x] [Refactor] XContent base classes from xcontent to core library (#5902) #6470
[Backport 2.x] [Refactor] XContent base classes from xcontent to core library (#5902) #6470
Conversation
Gradle Check (Jenkins) Run Completed with:
|
Codecov Report
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more @@ Coverage Diff @@
## 2.x #6470 +/- ##
============================================
+ Coverage 70.36% 70.38% +0.01%
- Complexity 59197 59261 +64
============================================
Files 4796 4796
Lines 284227 284205 -22
Branches 41304 41305 +1
============================================
+ Hits 200005 200046 +41
+ Misses 67523 67443 -80
- Partials 16699 16716 +17
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
@nknize I would very much like to backport this to avoid major divergence between the branches, but I'm having a hard time squaring this with our compatibility guarantees within a major version. Classes like |
That's fair! Maybe we give it a time limit for feedback so it doesn't get lost indefinitely? |
I am afraid this will make lots of people unhappy, even if it's a trivial change. But maybe we can help them by preparing the change for them? @nknize you could use meta with https://github.com/opensearch-project/opensearch-plugins and do a bulk change and submit a PR into each repo. Like so: https://github.com/opensearch-project/project-meta#make-a-change-in-many-repos |
Opened #6841 for tracking |
I'm going to give this until the evening then merge so downstream folks have the week to make their refactor changes in advance of feature freeze. @reta @andrross @saratvemulapalli are y'all good with this? |
👍 to me, I hope the migration for community plugin developers is going to be straightforward |
Thanks @nknize. I've opened up an RFC for Protobuf[1]. I dont think SDK has concerns, OpenSearch 2.x is SDK 1.x and we should just backport a PR which shouldn't be a problem. Tagging @dbwiddis for thoughts. |
👍🏻 . Plugins already have consumed these changes in 3.0 (a.k.a main) should be a backport to their 2.x branch, it shouldn't be invasive. I am worried for plugins externally which are not part of the project since its a breaking change, communicating is the best thing we could do. |
Here's one: #7728 |
Why is this not mentioned in the changelog? This was a breaking change! |
It should have been mentioned in the CHANGELOG. @nknize can you please add it? Regarding whether this was a breaking change in the semver sense, it's not. You can take a deep dependency on just about anything in core, and any change will break anyone that takes such a dependency, including a major upgrade of a dependent library. We ensure that all end-user features (e.g. RESTful interfaces) follow semver, but for java dependencies OpenSearch requires you to rebuild your plugin against every minor version. To solve this in core, @nknize is refactoring such that we can one day declare that |
Backport 9b9158e from #5902