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

Fix pr 14: fixes to swagger parser #21

Merged
merged 7 commits into from
Jun 20, 2020

Conversation

nathanvda
Copy link
Contributor

I created a new branch containing all the work in PR #14 and added a small fix to make sure the tests work.

I am not sure if this the correct way to go about it. I had to locally get the master branch from both
vincentvanbush and prograils to be able to 1) reproduce the error and 2) be able to fix it. My fix is very small, it is just this line:

  require 'active_record`

inside the db_record.rb file. Unfortunately due to my git inability (I am fine using git on a day to day basis, but still when actually git-fu is needed I am still lacking skillz) I inadvertently overwrote the author-name on the last commit.

So probably best is just apply the fix on top of PR 14? Proceed as you see fit.

When testing locally I also encountered some missing *_example methods so I

  • added the ones I missed
  • made sure future missing example methods do not crash, but issue some kind of warning? Not entirely sure how to best proceed with this?

If you fix PR14 instead (which is more honour to the original authors) I will open a new PR with my fixes applied (if that is ok).

vincentvanbush and others added 7 commits May 24, 2020 12:48
…ute keys

I did not author this code, I just fixed it. Maybe best to adapt the original
PR’s.
I add some example methods I missed, and made sure 
that future missing example methods do not crash the
normal operation, but instead show some kind of warning
so it could (hopefully) be fixed. 

I am aware that `puts` is probably not the best
way, but not sure what to use instead? `warn` ? 
Something silenceable?
@EmCousin EmCousin merged commit d10d8e4 into EmCousin:master Jun 20, 2020
@EmCousin
Copy link
Owner

Thank you @nathanvda, I merged #14 then fixed the conflicts then merged #21. That did the trick 👍

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