Switch to snake_case by convention for values #518
Conversation
This renames all builtins and standard library functions to conform to a snake case convention, as per decision recorded in #493. This also adapts examples and tests to this change. All functions have been renamed the way you'd expect, except the following: * `fromPred` -> `from_predicate` * `typeOf` -> `typeof` (like in C) * `fieldsOf` -> `fields` (the `_of` is redundant for a function) * `valuesOf` -> `values` (the `_of` is redundant for a function) Closes #493
| @@ -51,7 +51,7 @@ let OptLevel = fun label value => | |||
| contracts.blame label in | |||
|
|
|||
| let Contract = { | |||
| pathLibC | doc "Path to libc." | |||
| path_libc | doc "Path to libc." | |||
There was a problem hiding this comment.
Should we apply the change to configuration option? In real life, their name is dictated by the consuming tool (say, Terraform), most commonly in camelCase than in snake_case.
| @@ -11,7 +11,7 @@ let PortElt | |||
| | doc "A contract for a port element of a Kubernetes configuration" | |||
| = { | |||
| name | Str, | |||
| containerPort | #Port, | |||
| container_port | #Port, | |||
There was a problem hiding this comment.
Illustrating the previous comment, the field names in this example are taken from an actual Kubernetes schema, so we don't have the freedom of picking the case.
There was a problem hiding this comment.
I might have been trigger happy here. But again, I think it's best to have an explicit pointer to the schema, otherwise future folks like me won't know that the names were not picked freely. Do you have a URL handy?
There was a problem hiding this comment.
The official reference is https://kubernetes.io/docs/reference/kubernetes-api/. Another useful and better tailored resource is the original example it was inspired from (IIRC): https://github.com/kubernetes/examples/blob/master/guestbook-go/guestbook-controller.json.
|
|
||
| typeOf : Dyn -> < | ||
| typeof : Dyn -> < |
There was a problem hiding this comment.
Should we really follow C/C++ precedent here, though? For example, this random Rust crate uses type_of, which IMHO is more consistent with our and Rust's naming convention.
There was a problem hiding this comment.
The value of any foo applied to baz is the foo of baz. So any _of in a function is redundant and shouldn't be part of any naming convention. The only reason something like it is there in C/C++ and elsewhere is presumably because type is a reserved keyword. I saw the "random Rust crate" you mention, but with 480 downloads over its lifetime, it hardly has enough clout to set a meaningful precedent.
There was a problem hiding this comment.
Ok, fair enough. By the way type is currently not a reserved keyword in Nickel, and we've been trying to avoid to do so when possible (such as with type aliases) as it may easily appear in configurations and would be annoying to escape everywhere (e.g. in the current schema of NixOS modules). However it is not be good idea to tie our hands now and prevent any future use of a type keyword, so let's go with typeof.
There was a problem hiding this comment.
Thinking about this more: in C/C++, typeof is an operator. Probably has that name because no other keyword contains an underscore. In Python, the equivalent is a function called type(). Could we do the same here? type doesn't appear to be a reserved keyword yet and maybe we never want it to be. I have provisionally updated the PR accordingly.
There was a problem hiding this comment.
Hm, don't know what to make of the message timestamps above. You somehow answered my question from 14 minutes in the future. :)
I've kept typeof then. It's easier to go typeof -> type later, than the other way around.
There was a problem hiding this comment.
I actually answered from the future indeed 😛 I wrote my reply as an answer to your first comment without seeing the following ones.
There was a problem hiding this comment.
the fields exported in a configuration should not all be automatically capitalized in snake_case in the examples, I think.
I agree that configuration should use whatever schema is imposed by upstream. But in that case we should be explicitly referencing what schema we are assuming. API fields are no more commonly camelCase than snake_case IME. It really depends.
| @@ -11,7 +11,7 @@ let PortElt | |||
| | doc "A contract for a port element of a Kubernetes configuration" | |||
| = { | |||
| name | Str, | |||
| containerPort | #Port, | |||
| container_port | #Port, | |||
There was a problem hiding this comment.
I might have been trigger happy here. But again, I think it's best to have an explicit pointer to the schema, otherwise future folks like me won't know that the names were not picked freely. Do you have a URL handy?
|
|
||
| typeOf : Dyn -> < | ||
| typeof : Dyn -> < |
There was a problem hiding this comment.
The value of any foo applied to baz is the foo of baz. So any _of in a function is redundant and shouldn't be part of any naming convention. The only reason something like it is there in C/C++ and elsewhere is presumably because type is a reserved keyword. I saw the "random Rust crate" you mention, but with 480 downloads over its lifetime, it hardly has enough clout to set a meaningful precedent.
Add a pointer to original example.
|
@yannham PTAL |
In #518, I had forgotten to change the names of the primitives in `stdlib/strings.ncl`. I noticed the omission because this was breaking the example in the nickel-lang.org playground.
This renames all builtins and standard library functions to conform to
a snake case convention, as per decision recorded in #493. This also
adapts examples and tests to this change. All functions have been
renamed the way you'd expect, except the following:
fromPred->from_predicatetypeOf->typeof(like in C)fieldsOf->fields(the_ofis redundant for a function)valuesOf->values(the_ofis redundant for a function)Closes #493