Honor documented default: and stop reading route.desc - #990
Open
numbata wants to merge 5 commits into
Open
Conversation
The README spelling was a silent no-op because route.default is Hash#default. Read default_response first, then the :default key via route.settings. Drop route.desc so get '/', desc: is no longer treated as a swagger summary. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Danger ReportNo issues found. |
Same Style/DirectiveScope fix as #988; CI on this branch fails until those pairs are gone, and that PR has not landed on master yet. Co-authored-by: Cursor <cursoragent@cursor.com>
Grape 2.1.3 has no Grape::Util::ApiDescription, so evaluating DSL_METHODS at describe-load time raised NameError and failed the matrix row. Co-authored-by: Cursor <cursoragent@cursor.com>
Grape 3.0.1 has the class but not that constant, so the previous guard still raised NameError while loading the default: spec. Co-authored-by: Cursor <cursoragent@cursor.com>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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 #989. Follow-up from #987.
Summary
desc ..., default:but the code only ever readdefault_response:, so the documented spelling was a silent no-op. 2.3 reads both:route.try(:default_response)(the spelling that already worked) thenroute.settings.dig(:description, :default)(the README spelling, through a real reader).route.defaultcannot be the fallback — it isOrderedOptions/Hash#defaultand answersnileven when the:defaultkey is set. Preferdefault_response:;default:remains until 3.0.route.desc. Grape'sdescDSL stores:description.get '/', desc: '…'is a passthrough onto the route, not that DSL, and is no longer treated as a swagger summary. Usedesc '…'orsummary:.Test plan
default_response:unchanged (api_swagger_v2_response_with_models_spec.rband the primitive-types sibling).default:(skipped on Grape HEAD, which remaps that key and warns).desc '…'and assert swaggerdescription; a dedicated example pins thatget '/', desc:is not a summary.Made with Cursor