| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
Sorry, something went wrong.
I think ignoring leap years is not a good idea. |
Sorry, something went wrong.
Agreed, we can probably at least support leap years, like e.g. Java's java.time.LocalDate does. In general, we should not reinvent the wheel here and base the API and implementation of an established solution, like e.g. Java's JSR310 |
Sorry, something went wrong.
Yep, the current proposal is more based on the python datetime module. I wrongly made the assumption that they don't support leap years 🙁. I was just giving the wrong input with the year 2011 🤦 >>> datetime.datetime(2011, 2, 29)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
ValueError: day is out of range for monthTrying with an actual leap year works fine. >>> datetime.datetime(2024, 2, 29) |
Sorry, something went wrong.
|
Hi @darkdrag00nv2 - this FLIP is not reflected on FLIP project tracker. Did you follow the process outlined in https://github.com/onflow/flips? Specifically please remember to do the following without which the FLIP won't get visibility on the project tracker- Create an issue by using one of the FLIP issue templates based on the type of the FLIP - application, governance, cadence or protocol. The title of the issue should be the title of your FLIP, e.g., "Dynamic Inclusion fees". Submit the issue. Note the issue number that gets assigned. Thank you! |
Sorry, something went wrong.
There was a problem hiding this comment.
Looks good!
As we're finishing work on Cadence 1.0, we can finally come back to proposals like this. Thank you for your patience!
I left some comments and suggestions, and am happy to apply suggested edits and add remaining details. Does that sound good to you @darkdrag00nv2 ?
Once we addressed the outstanding issues, we can schedule to vote on it in the next WG call
Sorry, something went wrong.
|
|
||
| - `LocalDateTime.fromTimestamp(timestamp : UFix64)`: Creates a `LocalDateTime` with value based on the passed timestamp argument. | ||
|
|
||
| - `LocalDateTime.of(year: UInt64, month: Month, day: UInt8, hour: UInt8, minute: UInt8, second: UInt8)`: Creates a `LocalDateTime` with value based on the passed arguments assuming that there are 3600*24 seconds in every day. |
There was a problem hiding this comment.
This can maybe just be a constructor function. It might be good to improve the naming of day and make it explicit that it is the day of the month, e.g. dayOfMonth:
| - `LocalDateTime.of(year: UInt64, month: Month, day: UInt8, hour: UInt8, minute: UInt8, second: UInt8)`: Creates a `LocalDateTime` with value based on the passed arguments assuming that there are 3600*24 seconds in every day. | |
| - `LocalDateTime(year: UInt64, month: Month, dayOfMonth: UInt8, hour: UInt8, minute: UInt8, second: UInt8)`: Creates a `LocalDateTime` with value based on the passed arguments assuming that there are 3600*24 seconds in every day. |
Examples below also need to be updated
Sorry, something went wrong.
|
|
||
| 1. `getYear(): UInt32`: Get the year of the date-time. | ||
| 2. `getMonth(): Month`: Get the month of the date-time. | ||
| 3. `getDate(): UInt8`: Get the date of the date-time. |
There was a problem hiding this comment.
Maybe improve the naming and clarify it is the day of the month, i.e.
| 3. `getDate(): UInt8`: Get the date of the date-time. | |
| 3. `getDayOfMonth(): UInt8`: Get the day-of-month of the date-time. |
Examples below also need to be updated
Sorry, something went wrong.
| The `LocalDateTime` type will be defined as follows: | ||
|
|
||
| ```cadence | ||
| pub struct LocalDateTime { |
There was a problem hiding this comment.
The proposal needs to be updated to Cadence 1.0. For example:
Sorry, something went wrong.
|
|
||
| ## Design Proposal | ||
|
|
||
| New types `LocalDateTime` & `Duration` will be added to Cadence. Both of these data types assume that there are 3600*24 seconds in every day and there are no leap years. |
There was a problem hiding this comment.
I wonder if we should add this functionality as a standard library contract, i.e. namespace it and not add them to the global namespace.
Sorry, something went wrong.
| - `getDays(): Int32`: Get the days of the `Duration`. | ||
| - `getSeconds(): UInt32`: Get the second of the `Duration`. | ||
| - `getMicroseconds(): UInt32`: Get the microseconds of the `Duration`. | ||
| - `addYears(years : UInt32): Duration`: Return a new `Duration` after adding the provided number of years to it. |
There was a problem hiding this comment.
Here and for the other functions below: The function name already clarifies what the parameter is, so the argument label can be removed:
| - `addYears(years : UInt32): Duration`: Return a new `Duration` after adding the provided number of years to it. | |
| - `addYears(_ years: UInt32): Duration`: Return a new `Duration` after adding the provided number of years to it. |
Sorry, something went wrong.
| - `getDays(): Int32`: Get the days of the `Duration`. | ||
| - `getSeconds(): UInt32`: Get the second of the `Duration`. | ||
| - `getMicroseconds(): UInt32`: Get the microseconds of the `Duration`. | ||
| - `addYears(years : UInt32): Duration`: Return a new `Duration` after adding the provided number of years to it. |
There was a problem hiding this comment.
The add name sounds like it is mutating, but it seems like that isn't the case right?
Maybe we can use the plus and minus naming like in Java?
Sorry, something went wrong.
| dateTimeFromTs.getSecond() // 12 | ||
| dateTimeFromTs.getDayOfWeek() // wednesday | ||
|
|
||
| let dateTimeFromComponents = LocalDateTime.of(2019, april, 29, 19, 49, 31) |
There was a problem hiding this comment.
Need to add argument labels, as required by the function declaration
Sorry, something went wrong.
|
We had a working group call yesterday and discussed this FLIP, see the notes in https://github.com/onflow/Flow-Working-Groups/blob/main/cadence_language_and_execution_working_group/meetings/2024-10-29.md#flips. Consensus is that the proposed library would be useful, but is low priority. Luckily it can be implemented purely in Cadence, and does not need any knowledge of or required any contributions to the Cadence implementation. @onflow/flow-develop-experience-tools @gregsantos: @dete proposed this could be a good item for the community and could e.g. be implemented through a grant, at a hackathon, etc. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
#251
Work towards onflow/cadence#843