-
Notifications
You must be signed in to change notification settings - Fork 79
Generate beta and experimental APIs, guarded by Cargo features #161
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
Conversation
5a2aad1
to
47064c2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is awesome @swallez, thanks for adding!
I think there's a few more changes that should be made before merging, mostly related to documentation:
-
Add
beta-apis
andexperimental-apis
to the listed features in elasticsearch\lib.rs and the root README.md, and indicate what each means e.g. less stable, subject to change,experimental-apis
includes beta APIs, etc. -
Add
--all-features
totasks.docs
in Makefile.toml so that documentation includes all beta and experimental APIs -
Include wording in the generated documentation for beta and experimental APIs that indicates they are beta and experimental, respectively, and how to enable. For example, for beta, something like
/// # Optional /// This requires the `beta-apis` feature to be enabled
-
Add namespace markdown files in api_generator\docs\namespaces for
- autoscaling
- eql
- logstash
- monitoring
- rollup
- searchable_snapshots
- text_structure
-
Don't generate the
data_frame_transform_deprecated
namespace
#[cfg(feature = "beta-apis")] | ||
#[derive(Debug, Clone, PartialEq)] | ||
#[doc = "API parts for the Data Frame Transform Deprecated Delete Transform API"] | ||
pub enum DataFrameTransformDeprecatedDeleteTransformParts<'b> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The whole data_frame_transform_deprecated
namespace should never be generated. Looking through, I think it's the only exception.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point! Looking at the Rest spec for this, It seems we could have a rule that filters out deprecated endpoints that are not stable (this namespace is deprecated as the feature has been implemented slightly differently as stable endpoints). If it proves to require significant changes I'll hard-code the exclusion and open an issue to implement it post-7.11.
Thanks for the review @russcam. All good points, I'll work on this, and also on the yaml tests that keep failing ;-) |
@russcam I have addressed all your suggestions. Can you please cross-check?
|
6816139
to
e61cefd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - great effort on this PR, @swallez 🎉
Generates all APIs, including
beta
andexperimental
. Non-stable APIs are guarded by new Cargo featuresbeta-apis
andexperimental-apis
(which also includes beta).This allows applications to only have access to stable APIs (as previously) by default, and opt-in for unstable APIs, at their own risk since they can change in subsequent releases.
This also fixes the yaml-test-runner that relies on the
tasks
apis that were previously incorrectly marked as stable and are now experimental.