Skip to content
This repository was archived by the owner on Aug 4, 2025. It is now read-only.

Conversation

@trueheart78
Copy link
Contributor

In #22 the sinatra-contrib gem was replaced with sinatra, but that did not take into account that sinatra-contrib is required for the require 'sinatra/json' in lib/sinatra/stoplight_admin.rb. See the Sinatra::JSON (part of Sinatra::Contrib) for details.

gus - psych.gif

@bolshakov Could you take a look at this? Maybe there is a simple way to add specs to help with the altering of the required gems to keep things like this from happening in the future? I'd be glad to assist with that, if needed.

@bolshakov
Copy link
Owner

Yeah, we need at least smoke tests to be able to catch such simple cases. Let's add tests first and then merge your PR?

Yeah, we need at least smoke tests to be able to catch such simple cases. Let's add tests first and then merge your PR?

I'll try to add some tests on the weekend.

@trueheart78
Copy link
Contributor Author

Sounds like a plan. I might have time before then to work on it. Is there a preference for a particular testing framework? I'm fine with both minitest and rspec.

@bolshakov
Copy link
Owner

That would be very helpful! I'd suggest using rspec to be consistent with Spotlight

@trueheart78
Copy link
Contributor Author

This is ready for review. Feel free to make any changes that you might see fit. You can verify the spec passes if you change the sinatra-contrib line in the gemspec file to sinatra and re-bundle and re-run the spec.

I hope this helps out.

spongebob - hand wipe.gif

@trueheart78
Copy link
Contributor Author

@bolshakov I hope everything looks good. Will you have time soon to review?

@trueheart78
Copy link
Contributor Author

@bolshakov We're trying to finalize our Rails upgrade and I just wanted to check in again. I don't mean to pester, as I know you are busy.

@bolshakov bolshakov merged commit 7e6d24c into bolshakov:master Sep 22, 2019
@bolshakov
Copy link
Owner

@trueheart78 that's a good start, thanks 👍

@bolshakov
Copy link
Owner

@trueheart78 trueheart78 deleted the fix-sinatra-json-error branch May 22, 2021 21:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants