-
Notifications
You must be signed in to change notification settings - Fork 102
Openapi: adding more info in x-state, adding x-state info to fields and params #4431
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
There is info about the differences between these extensions in https://docs.bump.sh/help/specification-support/doc-badges/. The benefit of using x-beta was that it affected what was deemed a "breaking change" in the automated changelog. However, given that we didn't have the same consideration for tech preview / experimental states, IMO it's not worth trying to have both the
Since this PR affects only a specification extension, in my opinion it should be something that's ignorable by any other tools or processes that consume these OpenAPI documents. That is to say, IMO it can be standard output. |
I did a quick skim of the output and the
And here's what I see in this PR's serverless-specific output:
IMO the output should match the |
63774cd
to
0c62637
Compare
@lcawl fixed the serverless output! |
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.
Output LGTM, thanks!
62ba78e
to
a346e1a
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.
Left a some code style comments. format!()
is your friend for string formatting and even concatenation!
@@ -103,7 +104,7 @@ pub fn convert_expanded_schema(model: &IndexedModel, config: &Configuration) -> | |||
continue; | |||
} | |||
} | |||
paths::add_endpoint(endpoint, &mut tac, &mut openapi.paths)?; | |||
paths::add_endpoint(endpoint, &mut tac, &mut openapi.paths, &config.flavor)?; |
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 config is part of tac
, I added it in #4313. This effectively makes TypesAndComponents
more than types and components. I'll open a PR to rename it to something more "kitchensink-esque" like Context
.
let stab = stability.clone().unwrap_or(Stability::Stable); | ||
let mut since_str = "".to_string(); | ||
if let Some(since) = since { | ||
since_str = "; Added in ".to_string() + since; |
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 +
operator on String
is rarely used in Rust. Prefer the more compact and more efficient since_str = format!("; Added in {since}");
The backport to
To backport manually, run these commands in your terminal: # Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-9.0 9.0
# Navigate to the new working tree
cd .worktrees/backport-9.0
# Create a new branch
git switch --create backport-4431-to-9.0
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 6998d91c470b460f0cd6928bedb71027fa26696f
# Push it to GitHub
git push --set-upstream origin backport-4431-to-9.0
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-9.0 Then, create a pull request where the |
The backport to
To backport manually, run these commands in your terminal: # Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-8.18 8.18
# Navigate to the new working tree
cd .worktrees/backport-8.18
# Create a new branch
git switch --create backport-4431-to-8.18
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 6998d91c470b460f0cd6928bedb71027fa26696f
# Push it to GitHub
git push --set-upstream origin backport-4431-to-8.18
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-8.18 Then, create a pull request where the |
The backport to
To backport manually, run these commands in your terminal: # Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-8.19 8.19
# Navigate to the new working tree
cd .worktrees/backport-8.19
# Create a new branch
git switch --create backport-4431-to-8.19
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 6998d91c470b460f0cd6928bedb71027fa26696f
# Push it to GitHub
git push --set-upstream origin backport-4431-to-8.19
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-8.19 Then, create a pull request where the |
💚 All backports created successfully
Questions ?Please refer to the Backport tool documentation |
💚 All backports created successfully
Questions ?Please refer to the Backport tool documentation |
💚 All backports created successfully
Questions ?Please refer to the Backport tool documentation |
…nd params (#4431) (#4584) (cherry picked from commit 6998d91) Co-authored-by: Laura Trotta <[email protected]>
…nd params (#4431) (#4585) (cherry picked from commit 6998d91) Co-authored-by: Laura Trotta <[email protected]>
…nd params (#4431) (#4586) (cherry picked from commit 6998d91) Co-authored-by: Laura Trotta <[email protected]>
Closes #4386, Closes #4356.
Couple of doubts:
This would replace "x-beta" with "x-state" in the Beta case, and change the field type from a boolean to a string, is this okay?Does this also have to be configurable, or can it be the standard output?