martin-g commented on code in PR #2062:
URL: https://github.com/apache/avro/pull/2062#discussion_r1084117029
##########
lang/ruby/lib/avro/schema_validator.rb:
##########
@@ -208,11 +213,16 @@ def validate_union(expected_schema, datum, path, result,
options)
def first_compatible_type(datum, expected_schema, path, failures,
options = {})
expected_schema.schemas.find do |schema|
# Avoid expensive validation if we're just validating a nil
- next datum.nil? if schema.type_sym == :null
+ if schema.type_sym == :null
+ next datum.nil?
+ end
result = Result.new
validate_recursive(schema, datum, path, result, options)
- failures << { type: schema.type_sym, result: result } if
result.failure?
+
+ failure = { type: schema.type_sym, result: result }
+ failure[:schema] = schema.name if
schema.is_a?(Avro::Schema::RecordSchema)
Review Comment:
What about the other named schemas : Enum and Fixed ?
##########
lang/ruby/lib/avro/schema_validator.rb:
##########
@@ -208,11 +213,16 @@ def validate_union(expected_schema, datum, path, result,
options)
def first_compatible_type(datum, expected_schema, path, failures,
options = {})
expected_schema.schemas.find do |schema|
# Avoid expensive validation if we're just validating a nil
- next datum.nil? if schema.type_sym == :null
+ if schema.type_sym == :null
+ next datum.nil?
+ end
result = Result.new
validate_recursive(schema, datum, path, result, options)
- failures << { type: schema.type_sym, result: result } if
result.failure?
+
+ failure = { type: schema.type_sym, result: result }
+ failure[:schema] = schema.name if
schema.is_a?(Avro::Schema::RecordSchema)
+ failures << failure if result.failure?
Review Comment:
IMO it would be cleaner if the whole block is wrapped in `if`, as the change
you made above (L216).
There is no need to create `failure` object if `result.failure?` is `false`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
