-
Notifications
You must be signed in to change notification settings - Fork 385
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: remove monoclock & add tz as chain param #3193
base: master
Are you sure you want to change the base?
feat: remove monoclock & add tz as chain param #3193
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅ 📢 Thoughts on this report? Let us know! |
Hi @moul, I see two ways to make a constant parameter as discussed in the last contributor call
I see an advantage in the second option, which is that you can easily change a parameter from const to non-const, whereas in the first option the timezone won't be modifiable regardless of the configuration. Now, do we want the tz to be modifiable on certain chains? If you see a third or more options, don't hesitate. |
🛠 PR Checks SummaryAll Automated Checks passed. ✅ Manual Checks (for Reviewers):
Read More🤖 This bot helps streamline PR reviews by verifying automated checks and providing guidance for contributors and reviewers. ✅ Automated Checks (for Contributors):🟢 Maintainers must be able to edit this pull request (more info) ☑️ Contributor Actions:
☑️ Reviewer Actions:
📚 Resources:Debug
|
I was also considering adding a global variable to the standard library that is set to a custom value during its What do you think? |
Yeah i think it could be a good idea, at least for now and make it easier to implement. |
…dd-tz-as-chain-param
…dd-tz-as-chain-param
based on #3016
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 variadic timezone, and now an instance of Time is composed as follows:
The timezone is now a const defined in
timezoneinfo.gno
seevar chainLoc Location = Location{name: "UTC"}
When I parse a date with a different timezone, it's automatically transformed into the chain timezone.
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 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 always use the chain tz)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.
Contributors' checklist...
BREAKING CHANGE: xxx
message was included in the description