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.

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

civil: add Compare method to Date, Time and DateTime #10009

Closed
shota3506 opened this issue Apr 20, 2024 · 5 comments · Fixed by #9775
Closed

civil: add Compare method to Date, Time and DateTime #10009

shota3506 opened this issue Apr 20, 2024 · 5 comments · Fixed by #9775
Assignees
Labels
type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@shota3506
Copy link
Contributor

Is your feature request related to a problem? Please describe.

This proposal suggests adding Compare method to civil.Date, civil.Time and civil.DateTime

Some structs in the standard package have Compare method which returns a integer value of either -1, +1, or 0.
https://pkg.go.dev/time#Time.Compare
https://pkg.go.dev/net/netip@master#Addr.Compare

It looks like the standard way to define ordered objects in Go (golang/go#50770 (comment)), and it is useful when we sort such objects by slices.SortFunc.

Describe the solution you'd like
I'll open a PR for this issue soon.

Describe alternatives you've considered
Of course, we can judge the order of civil objects by using exiting After and Before.
If there are opinions that it is unnecessary, I will respect them.

Additional context

@shota3506 shota3506 added the triage me I really want to be triaged. label Apr 20, 2024
@noahdietz noahdietz added type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. and removed triage me I really want to be triaged. labels Apr 22, 2024
@noahdietz
Copy link
Contributor

Just reviewed the PR.

Thanks for the idea/contribution.

@noahdietz noahdietz linked a pull request Apr 22, 2024 that will close this issue
@noahdietz
Copy link
Contributor

@shota3506 sorry, need to hold onto this for a little bit. We want to upgrade to Go 1.20 soon. Please reopen your PR after the reversion goes through and we will merge it as soon as we upgrade!

@shota3506
Copy link
Contributor Author

shota3506 commented Apr 22, 2024

@noahdietz Thank you for your review and comment!
I haven't checked go directive in this project sorry...
I'll wait for the version upgrade 👍

@shota3506
Copy link
Contributor Author

@noahdietz

Hi ✋
The minimum version has been upgraded to 1.20, right?

I've reopened the PR #10193
Please check when you have time. Thanks.

@noahdietz
Copy link
Contributor

Yes @shota3506 good eye :) Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants