Runs code formatter on NaiveDateTime #6889
Conversation
| raise ArgumentError, | ||
| "cannot calculate the difference between #{inspect(naive_datetime1)} and #{ | ||
| inspect(naive_datetime2) | ||
| } because their calendars are not compatible and thus the result would be ambiguous" |
There was a problem hiding this comment.
this break feels a little strange to me, but the only way I could come up to avoid it was to concatenate multiple strings. any comments?
There was a problem hiding this comment.
Please do split the message into multiple strings and concatenate them with <>. It's what we do all over the codebase 👍
There was a problem hiding this comment.
Done! Thank you for reviewing it!
| minute: minute, | ||
| second: second, | ||
| microsecond: microsecond | ||
| }) do |
There was a problem hiding this comment.
Would it be better to change this to the following?
def to_string(naive_datetime) do
%{calendar: calendar,
year: year,
month: month,
day: day,
hour: hour,
minute: minute,
second: second,
microsecond: microsecond} = naive_datetime
calendar.naive_datetime_to_string(year, month, day, hour, minute, second, microsecond)
endIf changed and a map/struct not matching this keys is passed to the function, it would change the expected error. I felt like keeping the keys there made more sense.
comments?
There was a problem hiding this comment.
Yes please keep the keys in order to leave the semantics intact. We can always revisit later if we want to do more "refactory" changes.
nirev
force-pushed
the
format-naive-datetime
branch
from
fcf2a39 to
d41034d
Compare
nirev
force-pushed
the
format-naive-datetime
branch
from
d41034d to
30d2933
Compare
|
❤️ 💚 💙 💛 💜 |
Hopefully it is all good :)