choptastic / qdate

Erlang date, time, and timezone management: formatting, conversion, and date arithmetic
http://sigma-star.com/blog/post/qdate
MIT License
246 stars 82 forks source link

Fix/preserve millsec #41

Closed leonardb closed 3 years ago

leonardb commented 3 years ago

Option to preserve milliseconds in date parsing and formatting

choptastic commented 3 years ago

This is great! Nice clean code with tests, even! Thank you!

leonardb commented 3 years ago

@choptastic Sorry with the late comment I was away after submitting the PR.

I thought I should ask if there is a better or preferred method for accessing the option rather than calling application:get_env/3 with every operation.

I'd noticed you placing some options into the process dictionary, but it was not clear if following that pattern was a 'better' method or not.

choptastic commented 3 years ago

That's a good question. I think it's probably fine, though if more and more options like this get added, it may be worth refactoring to add an actual Options argument to many of the functions. That, however, feels a little overkill.

The majority of the process dictionary stuff has to do with specific processes needing settings that my only apply to that process (the classic example is setting a timezone at the process level for different users connected to a web service).

In this situation, I suspect if you need sub-second accuracy, you might need universally, rather than at the process level.

That said, if there is a need where a user might need the sub-second accuracy, while others using the same system do not (or it would add confusion), then yes, it would be worth rejiggering to use the process dictionary.

I appreciate the thoughtfulness in this. I think for now, we're probably fine.