Conversation
8fb59ec to
a577b1c
Compare
vitaut
left a comment
There was a problem hiding this comment.
The formatter should be conditionally defined since zoned_time is a C++20 feature.
|
Hi @vitaut , yes I've checked CI logs and is trying to solve it |
2a9839b to
e10e922
Compare
6787974 to
94381c8
Compare
|
Now it should be fine. |
|
Hi @vitaut , can you please review once again, when you have a time of course. Thanks) |
94381c8 to
b43a520
Compare
b43a520 to
93c76b5
Compare
ba17df2 to
8c1889b
Compare
vitaut
left a comment
There was a problem hiding this comment.
Overall looks good but please add a unit test to chrono-test.cc.
|
0a98a90 to
12aade3
Compare
|
Hi @vitaut , tests are added. Hope they are ok. |
12aade3 to
40ed39d
Compare
vitaut
left a comment
There was a problem hiding this comment.
Looks like there are some CI failures.
Yeah, had not time to check . Will try propably tomorrow |
Yeah, had not time to check . Will try propably tomorrow |
| if constexpr (detail::has_tm_zone<std::tm>::value) { | ||
| t.tm_zone = time_info.abbrev.c_str(); | ||
| t.tm_gmtoff = time_info.offset.count(); | ||
| } |
There was a problem hiding this comment.
I think we should only provide this specialization if tm supports time zones. Otherwise the timezone information will be ignored. This is basically why the tests are failing.
|
@LukashonakV do you still plan to update this PR? |
|
Hi @vitaut , sure. Apologies for the delay. A bit busy with main activities. Hope next week will be able to check. Actually I saw the problem and was wondering about the solution.. |
|
No hurry, just wanted to check if we should keep this PR open. Since timezone names are system-specific I think it would be fine to limit tests to Linux (and we probably don't need to test so many of them, one or two should be sufficient). |
Hi @vitaut , this PR provides zoned_time formatter.
With the result: