Skip to content

Swagger Error and Note Support - #113

Merged
bajtos merged 3 commits into
masterfrom
feature/error-notes
Oct 16, 2014
Merged

Swagger Error and Note Support#113
bajtos merged 3 commits into
masterfrom
feature/error-notes

Conversation

@ritch

@ritch ritch commented Oct 9, 2014

Copy link
Copy Markdown
Member

A follow-up for #110.

/to @bajtos

@ritch ritch added the #review label Oct 9, 2014
@bajtos

bajtos commented Oct 10, 2014

Copy link
Copy Markdown
Member

I have a question: why are notes handled differently than errors? I.e. notes are pass-through and may be undefined, errors are always at least an empty array.

Other than that, the patch LGTM. The meat of the feature is in loopback-explorer anyways (strongloop/loopback-component-explorer#52).

@bajtos

bajtos commented Oct 14, 2014

Copy link
Copy Markdown
Member

The explanation is that notes is a value that is either a string or an array of strings - see strongloop/loopback#638.

errors is an array of Swagger's Response Message Object.

The patch LGTM, please back-port it to strong-remoting 1.x too.

I am wondering if we should take a different approach and consider all properties on the options object as pass-through? It will save us from landing changes like this one in the future. Anyways, it's out of the scope of this PR.

@bajtos bajtos assigned bajtos and ritch and unassigned bajtos Oct 14, 2014
bajtos added a commit that referenced this pull request Oct 16, 2014
@bajtos
bajtos merged commit 473273c into master Oct 16, 2014
@bajtos bajtos removed the #review label Oct 16, 2014
@bajtos
bajtos deleted the feature/error-notes branch October 16, 2014 13:07
@bajtos
bajtos restored the feature/error-notes branch October 16, 2014 13:07
bajtos pushed a commit that referenced this pull request Oct 16, 2014
This is a backport of #113 from the master (2.x).
@bajtos

bajtos commented Oct 16, 2014

Copy link
Copy Markdown
Member

Released as v2.5.0 and v1.6.0 via #115

@ritch
ritch deleted the feature/error-notes branch October 17, 2014 17:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants