-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Add File.touch and FileUtils.touch methods #4069
Conversation
src/file.cr
Outdated
unless exists?(filename) | ||
File.open(filename, "a") { } | ||
end | ||
time ||= Time.now |
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.
Why not put = Time.now
as the default argument?
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.
Since this way you might still pass a nil
as an argument.
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.
Why would someone pass nil
?
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.
I have no strong opinion about it, might as well change it to = Time.now
.
src/file_utils.cr
Outdated
# ``` | ||
# FileUtils.touch(["foo", "bar"]) | ||
# ``` | ||
def touch(paths : Enumerable(String), time : Time? = nil) |
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.
Maybe a *paths
version?
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.
Sounds good, though I'm just following convention from FileUtils
here.
Ping? Anything actionable here? |
@Sija I think the removal of the support for |
315eaf5
to
6690f74
Compare
@luislavena 'fair nuff, done! |
Is this PR is ready to 🚢 ? |
So, should we redo some changes applied on #3986 since there is a built in touch? |
@bcardiff its usage is part of a broader |
@Sija yes, you are right. |
File feels like the right place for this, thank you for fixing another ruby oddity :) |
As in the title. Low hanging fruit since
File.utime
is now in stdlib.