Fix: OpenAPI warnings #749
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
newstler
force-pushed
the
fixes/openapi-warnings
branch
from
ed77b9b
to
3e57e6d
Compare
newstler
force-pushed
the
fixes/openapi-warnings
branch
from
538b8ce
to
82336f1
Compare
newstler
force-pushed
the
fixes/openapi-warnings
branch
2 times, most recently
from
ab42683
to
fc9d501
Compare
Merged
newstler
force-pushed
the
fixes/openapi-warnings
branch
from
fc9d501
to
a1ba4c5
Compare
|
There are some test failures in the joint PR, which also show up here in the |
newstler
force-pushed
the
fixes/openapi-warnings
branch
from
a1ba4c5
to
cc43de6
Compare
Gemfile.lock
Outdated
| @@ -605,7 +601,6 @@ GEM | |||
| PLATFORMS | |||
| arm64-darwin-21 | |||
| arm64-darwin-22 | |||
| ruby | |||
There was a problem hiding this comment.
@newstler, I'm curious about all the removals in Gemfile.lock. Was that intentional?
There was a problem hiding this comment.
@jagthedrummer Most probably not, the only thing that should have changed there is update of jbuilder-schema to 2.2.0. I just restored Gemfile.lock from main, updated jbuilder-schema again and rebased everything against main. Doesn't fail on my local machine
There was a problem hiding this comment.
👍 Excellent! The Release: failures here are just saying that these changes would break against the versions of the core gems that are currently pinned in Gemfile.lock. I'll merge the related PR in core, then release a new version of the gems, and bump Gemfile.lock and then this should be all good. Thanks for your work on it!
… to use it and to fail on warnings.
newstler
force-pushed
the
fixes/openapi-warnings
branch
from
cc43de6
to
40bafef
Compare
jagthedrummer
approved these changes
Aug 24, 2023
jagthedrummer
added a commit
that referenced
this pull request
Aug 24, 2023
In #749 we tried to bump the version of `jbuilder-schema`, but since it's a dependency of `bullet_train-api` we need to leave it alone and let it get upgraded naturally when we bump that gem.
jagthedrummer
added a commit
that referenced
this pull request
Aug 24, 2023
In #749 we tried to bump the version of `jbuilder-schema`, but since it's a dependency of `bullet_train-api` we need to leave it alone and let it get upgraded naturally when we bump that gem.
Closed
jagthedrummer
added a commit
that referenced
this pull request
Aug 24, 2023
In #749 we introduced more stringent checks in validating the Open API doc. https://github.com/bullet-train-co/bullet_train/pull/749/files#diff-c04c1d7b8b2c28801ca5fc5cfa976e08c1f9731284f8ad655655b2a12d9bcef6 After merging that, I also merged bullet-train-co/bullet_train-core#257 and #737 which were both passing prior to merging. I think that they probably would have failed that new test if their branches had been up to date. Since failing the tests if the doc is invalid is a brand new introduction, I'm going to disable those assertions in that test.
jagthedrummer
added a commit
that referenced
this pull request
Aug 24, 2023
In #749 we introduced more stringent checks in validating the Open API doc. https://github.com/bullet-train-co/bullet_train/pull/749/files#diff-c04c1d7b8b2c28801ca5fc5cfa976e08c1f9731284f8ad655655b2a12d9bcef6 After merging that, I also merged bullet-train-co/bullet_train-core#257 and #737 which were both passing prior to merging. I think that they probably would have failed that new test if their branches had been up to date. Since failing the tests if the doc is invalid is a brand new introduction, I'm going to disable those assertions in that test.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #734
Required by bullet-train-co/bullet_train-core#270
.redocly.yamlconfig to turn offno-ambiguous-pathscheck, as Redocly team contributor confirmed that some paths in our API are considered non ambiguous (/projects/{project_id}/goalsand/projects/tags/{id}for example).test/controllers/api/open_api_controller_test.rbso it fails on any warnings.