Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Where should Time#to_date and Time#to_datetime be? #1146

Closed
nobu opened this issue Apr 3, 2024 · 5 comments · Fixed by #1147
Closed

Where should Time#to_date and Time#to_datetime be? #1146

nobu opened this issue Apr 3, 2024 · 5 comments · Fixed by #1147

Comments

@nobu
Copy link
Member

nobu commented Apr 3, 2024

Currently, "specs" of these methods are placed under ruby/library/time.
But these are defined by the "date" library, which just extends Time.
It doesn't look the right place.
@eregon
Copy link
Member

eregon commented Apr 3, 2024

How about library/date/time/to_date_spec.rb / library/date/time/to_datetime_spec.rb ?
That seems consistent with e.g. https://github.com/ruby/spec/tree/master/library/prime/integer.
Could you commit that here or in ruby/ruby? (whatever is more convenient for you)

@nobu
Copy link
Member Author

nobu commented Apr 4, 2024

How about library/date/time/to_date_spec.rb / library/date/time/to_datetime_spec.rb ?

Isn't the latter library/datetime/time/to_datetime_spec.rb?

nobu added a commit to nobu/rubyspec that referenced this issue Apr 4, 2024
@nobu
Move the extensions for Time to the corresponding locations
e13e221 
Although Time#to_date and Time#to_datetime are defined in Time, they
belong to the date library and should be placed in the corresponding
locations for each class.

Fix ruby#1146.
nobu added a commit to nobu/rubyspec that referenced this issue Apr 4, 2024
@nobu
Move the extensions for Time to the corresponding locations
d29c935 
Although `Time#to_date` and `Time#to_datetime` are defined in `Time`,
they belong to the "date" library and should be placed in the
corresponding locations for each class.

Fix ruby#1146.
@nobu nobu mentioned this issue Apr 4, 2024
Merged
@hsbt hsbt closed this as completed in #1147 Apr 4, 2024
@eregon
Copy link
Member

eregon commented Apr 4, 2024 •
edited
Loading

Isn't the latter library/datetime/time/to_datetime_spec.rb?

No, because there is no require 'datetime'. library/X means the library which can be required with require 'X'

@eregon
Copy link
Member

eregon commented Apr 4, 2024

Ah but there is already library/datetime, so an existing issue, so fine as you did.

@eregon
Copy link
Member

eregon commented Apr 4, 2024

Probably we should have library/date/{date,datetime,time}, but I'm not sure how much it matters.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Move the extensions for Time to the corresponding locations nobu/rubyspec
2 participants
@nobu @eregon