User definable list of ignored errors - #98
Open
multiscan wants to merge 4 commits into
Open
Conversation
this includes a slight refactoring of the way the IGNORED_ERRORS array is built. The original code were allowing for class objects to be passed. The feature is not used because it can actually lead to exception when the class is not defined. Instead of a loop lookup, I think it is faster to rely on the keys of an hash.
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
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.
This PR enable the user to configure a list of error names that will be added to the hard-coded default list of ignored errors that is defined in
lib/solid_errors/subscriber.rb.A minor refactoring of the way IGNORED_ERRORS is built is also introduced for efficiency and compatibility reasons: the current approach leaves the option to use classes instead of class names but it might lead to errors in case the class is not defined. Since only class names are actually present in the array, I think it is better to stick to them and simplify the lookup by using an hash (~2 times faster than array.include? on my machine for this array size).
A small fix is also proposed for the
Occurrencemodel to imcrease compatibility with database not supporting direct json serialization. This is so tiny that I think a separate PR would be overkill.