-
Notifications
You must be signed in to change notification settings - Fork 375
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(stdlibs): simplify time pkg & remove tz + monoclock logic related #3016
base: master
Are you sure you want to change the base?
feat(stdlibs): simplify time pkg & remove tz + monoclock logic related #3016
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3016 +/- ##
==========================================
- Coverage 63.80% 63.75% -0.05%
==========================================
Files 549 549
Lines 78833 78833
==========================================
- Hits 50300 50262 -38
- Misses 25144 25176 +32
- Partials 3389 3395 +6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks good globally. However, after discussing with Jae which parameters should be specified in the new x/params module (#3003), we believe it would be beneficial to include a "global chain default timezone" to accommodate more regional chains. We can apply the same logic of markdown-based conversion. Instead of having no timezone support or encouraging per-contract timezone support, we ideally want to implement per-chain timezone support.
I understand and agree with you, i will look to adapt the PR to set the default tz as a parameter of the chain handle by x/params |
fix #851
The purpose of this Pull Request is to simplify the Time pkg, which comes from Go, in order to remove unnecessary functions for Gno.
This Pull Request removes the concept of monotonic clock, which corresponds to the time since the start of the program, often used to calculate the time interval between two events more reliably.
I've also removed the notion of timezone, and now an instance of Time is composed as follows:
When I parse a date with a different timezone, it's automatically transformed into UTC.
However, the goal would be to allow a Time instance to be displayed in a specific timezone with a markdown pattern, especially for Gnoweb.
Most of the PR is code deletion, not addition, especially the bitwise logic. see article
I've also adapted some unnecessary methods to avoid breaking realms, such as the Zone method, which returns the tz name and offset before the realm name, and is used in the
events
realm.I also kept some unused param like Location in Date to validate the CI & keep same API as Go:
In the code snippet above, i kept the
loc
parameter even if i don't use it anymore.I'd like to take this opportunity to say that I haven't answered the testing question, because the PR seems tedious enough to review as it is. If this PR ends up being merged, I'm thinking of doing a second PR for the testing part, to keep things as atomic as possible.
Contributors' checklist...
BREAKING CHANGE: xxx
message was included in the description