Fix issue 5 #13
Merged
Fix issue 5 #13
Conversation
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
Fixes jsonapi-suite#5 `json_include` specifying an index where nothing exists was the underlying problem. I have chosen to throw an error in that case, because nobody ought to be *trying* to get `nil` from that method. Since we are testing, a nice error message would probably be helpful. I also added an error class so that expectations in this repo testing that an error is thrown when it should be are less likely to give a false positive.
richmolj
reviewed
Feb 24, 2018
There was a problem hiding this comment.
Thanks for taking care of this @happycollision, very much appreciate the help! Had one minor comment then this is good to go 👍
lib/jsonapi_spec_helpers/error.rb
Outdated
| @@ -0,0 +1,2 @@ | |||
| class JsonapiSpecHelpersError < StandardError | |||
There was a problem hiding this comment.
Could we make this
module JsonapiSpecHelpers
module Errors
class Base < StandardError
class IncludeOutOfBounds < Base
end
endTo set up a generic error handling pattern for the future? This is similar to what we have in compliable.
There was a problem hiding this comment.
All set. I am pretty inexperienced with Ruby, so I am glad you directed me to a better way of dealing with errors. I will be using that pattern in my own project as needed.
Sets up a module in the same fashion as the JsonapiCompliable gem
|
Thanks again @happycollission! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
See my thoughts in my last commit. I know this would have been massively helpful for me, so I chose to fix it in this way. I am open to changes.