Skip to content
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

Allow extension keys in Info Object #588

Merged
merged 4 commits into from
Mar 1, 2017

Conversation

mattyr
Copy link
Contributor

@mattyr mattyr commented Feb 28, 2017

Copy link
Member

@LeFnord LeFnord left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @mattyr … but

  • make rubocop happy → run bundle exec rake to see the failures
  • missing changelog entry
  • please add a entry to extension section in the README

- make rubocop happy
- add changelog entry
- add entry to extension section in the README
@mattyr
Copy link
Contributor Author

mattyr commented Mar 1, 2017

made the requested changes -- also made the extension format conform better with existing extensions (x: {foo: 'bar'} rather than "x-foo": 'bar').

quick question re: rubocop: i was getting a ton of errors so i thought that was ignored. looks like 99% are "Missing frozen string literal comment". I disabled that using .rubocop.yml file and just addressed those issues specific to these changes. Would you like a PR that either:

  • disables the "Missing frozen string literal comment" test universally, or
  • adds the comment to all needed files (not sure if this will introduce new spec failures)

@coveralls
Copy link

coveralls commented Mar 1, 2017

Coverage Status

Coverage increased (+2.4%) to 99.754% when pulling dea6383 on mattyr:allow-extensions-in-info into 79052e3 on ruby-grape:master.

@LeFnord
Copy link
Member

LeFnord commented Mar 1, 2017

@mattyr … its a bit curious that this cop pops up, cause it is running some weeks, but prefer not to mix up your PR with a maintainance PR, and yeah here is it green

Copy link
Member

@LeFnord LeFnord left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 very cool, thanks

@LeFnord LeFnord merged commit 86dba46 into ruby-grape:master Mar 1, 2017
@coveralls
Copy link

Coverage Status

Coverage increased (+2.3%) to 99.631% when pulling dea6383 on mattyr:allow-extensions-in-info into 79052e3 on ruby-grape:master.

LeFnord pushed a commit to LeFnord/grape-swagger that referenced this pull request Feb 9, 2019
* Allow extension keys in Info Object, per http://swagger.io/specification/#infoObject

* address comments

- make rubocop happy
- add changelog entry
- add entry to extension section in the README

* fix changelog entry

* another changelog fix
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.

None yet

3 participants