Bikeshed for method naming conventions
There has been various changes in naming for methods over the lifetime of the development of temporal_rs, specifically around the use of snake case in methods and file names. The intended goal of this issue is to come to a consensus moving forward for naming conventions in temporal_rs.
So the first question to answer should probably be:
- Should snake case be adhered to 100% of the time regardless of method length?
- Should this naming convention also then be extended to filenames and modules?
- Should "Plain" be preserved in method names: "to_date" vs.
to_plain_dateor "to_time" vs. "to_plain_time"
General thoughts
My general thought on this subject is that naming in general is a question of to what degree snake case should be preserved in the general API. For instance, ZonedDateTime as a type is fine for a struct name, but can create some rather long and unyielding method names that may not make the best API if expanded to full snake case, e.g. to_zoned_date_time. In order to avoid long snake case methods that may make methods hard to deal with, ZonedDateTime has been condensed into one "proper" noun of sorts, e.g. to_zoneddatetime.
The issue with doing this type of naming is how it extends to other methods:
-
to_zoned_date_timevs.to_zoneddatetime -
to_plain_date_timevsto_plaindatetime -
to_plain_timevsto_plaintime
There are a few general names that have gone through various names and are in scope in connection to this issue (see below).
Time Zone
-
tz,timezone, ortime_zone
Time zone has changed in various points.
PlainDateTime / DateTime
-
datetime,plaindatetime,date_time,plain_date_time
ZonedDateTime
-
zoneddatetime,zoned_date_time
General thoughts?
Also, if anyone has any other thoughts regarding to this issue that I may have missed feel free to let me know
datetime,plaindatetime,date_time,plain_date_time
I'd suggest avoiding datetime nor date_time. Timezone-less date/time types are prone to bugs caused by programmers assuming that the offset will not change. By naming a zoneless type datetime it implies that the prefix-less type is the default type that should be used. In Temporal, we added the prefix "Plain" to make it clear that the developer needs to make a choice about what date/time type they want, rather than choosing a default-looking type.
we added the prefix "Plain" to make it clear that the developer needs to make a choice about what date/time type they want, rather than choosing a default-looking type.
That's a good call out. The goal at this point, at least from my perspective, is to mimic the Temporal specification API as close as possible. So if there is a method called toPlainDateTime, then we would translate it to to_plain_datetime/to_plain_date_time.
This issue now that I think about it may a bit overloaded. Part of the issue with our naming convention is we have parameters and modules named various things that are not aligned (mostly due to shifting opinions on my part of what is probably better). So is it best to snake case EVERYTHING or only certain things.
For instance, the example above. Should we treat DateTime as a unit or separate, e.g. date_time vs. datetime. This then extends to other APIs. to_plaindatetime vs. to_plain_date_time vs. to_plain_datetime or to_zoneddatetime vs. to_zoned_date_time vs. to_zoned_datetime.
It's also worth noting that this principle would also be extended to module names as well. Currently our module for ZonedDateTime is zoneddatetime. But should what should that be updated to full snake case.
I know this is EXTREMELY semantic, but I'd ideally like to have one unified approach for temporal_rs. Was there ever any discussion regarding whether a PlainDateTime could be interpreted as a plain datetime vs. a plain date time?
I know this is EXTREMELY semantic, but I'd ideally like to have one unified approach for temporal_rs. Was there ever any discussion regarding whether a
PlainDateTimecould be interpreted as a plain datetime vs. a plain date time?
Nope. Although you could infer from the PascalCased PlainDateTime that it would have been PlainDatetime if we'd considered "datetime" to be one word. So I'd assume that plain_date_time is probably OK for Rust.