-
Notifications
You must be signed in to change notification settings - Fork 160
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
Define STRFTIME/MOMENT/MOMENT-MS/RSS-DATE #163
base: master
Are you sure you want to change the base?
Conversation
I wanted to get another set of eyes on this before merging. |
My eyes are not very useful when there aren't tests to run. But these are new functions so no worries. I assume you're running them in laarc? Please remember to backport bugfixes as you run into them. Oh also, it may be convenient to add some |
@@ -648,6 +648,10 @@ More convenient form of [[coerce]] with arguments reversed; doesn't need | |||
"Converts 'x' into a symbol." | |||
(coerce x 'sym)) | |||
|
|||
(def str (x) |
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.
Do we need this, when we have string
?
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.
Possibly. (coerce x 'string)
behaves differently from (+ "" x)
.
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.
In what situation?
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.
In no situations. Never mind, I was mistaken. :)
One benefit of adding a str
function is that it can be a place to do things like printing a hash table using curly brace syntax. But that's work for some other time. I'll get rid of str
.
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.
Also, feel free to overload string
for those purposes.
(I don't particularly what the function is called. But it seems good to have one strong default.)
Yup! Previously I was shelling out to
No need to port bug fixes when the code is flawless. /s (Good point! I'll be sure to port any fixes that come up.)
Good idea! |
(Also define
str
andleftpad
)