zcsizmadia commented on a change in pull request #1623:
URL: https://github.com/apache/avro/pull/1623#discussion_r835809840
##########
File path: lang/csharp/src/apache/main/Util/TimeMicrosecond.cs
##########
@@ -50,17 +51,16 @@ public override object ConvertToBaseValue(object
logicalValue, LogicalSchema sch
{
var time = (TimeSpan)logicalValue;
- if (time > _maxTime)
+ if (time >= _maxTime)
throw new ArgumentOutOfRangeException(nameof(logicalValue), "A
'time-micros' value can only have the range '00:00:00' to '23:59:59'.");
Review comment:
The changes to you mention above makes sense. Lets mark this PR as 1.12
version and the dependent PR as well. In that case I feel more comfortable to
introduce potentially painful changes to end the end user. E.g changed
exception message or additional exceptions in case of incorrect values.
Other option to keep the scope of ticket as os and let's open a ticket for
the changes mentioned above and we will mark that ticket as as 12.0.
--
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]
