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

time: package has two different arrays with long month names #36359

Closed
petar-dambovaliev opened this issue Jan 2, 2020 · 4 comments
Closed

time: package has two different arrays with long month names #36359

petar-dambovaliev opened this issue Jan 2, 2020 · 4 comments
Labels
Milestone

Comments

@petar-dambovaliev
Copy link
Contributor

@petar-dambovaliev petar-dambovaliev commented Jan 2, 2020

What version of Go are you using (go version)?

go version go1.13.1 linux/amd64

Seems to me that there doesn't need to be code duplication here.
Do you mind if i remove it?

https://github.com/golang/go/blame/master/src/time/time.go#L288
https://github.com/golang/go/blame/master/src/time/format.go#L321

@gopherbot gopherbot added this to the Proposal milestone Jan 2, 2020
@gopherbot gopherbot added the Proposal label Jan 2, 2020
@ALTree ALTree changed the title proposal: std time time: package has two different arrays with long month names Jan 2, 2020
@ALTree ALTree added NeedsDecision and removed Proposal labels Jan 2, 2020
@ALTree ALTree modified the milestones: Proposal, Backlog Jan 2, 2020
@ALTree
Copy link
Member

@ALTree ALTree commented Jan 2, 2020

For something this small, I would just send a patch. It'll get reviewed (and accepted, or rejected) directly on gerrit.

@mariush-github
Copy link

@mariush-github mariush-github commented Jan 2, 2020

While you're in format.go , the shortDayNames array (line 296) could also be removed, if you reuse the longDayNames and take the first 3 characters - in this case it happens to match for all days.

Same for shortMonthNames array, seems like all 3 letter months have the same first 3 letters as in longMonthNames

also time.go: 326 there's a days array , format.go : 286 has longDayNames array

@petar-dambovaliev
Copy link
Contributor Author

@petar-dambovaliev petar-dambovaliev commented Jan 3, 2020

While you're in format.go , the shortDayNames array (line 296) could also be removed, if you reuse the longDayNames and take the first 3 characters - in this case it happens to match for all days.

Same for shortMonthNames array, seems like all 3 letter months have the same first 3 letters as in longMonthNames

also time.go: 326 there's a days array , format.go : 286 has longDayNames array

Cool, i will get on it.
Although, i would be against removing the short names. It would cause runtime overhead.

@gopherbot
Copy link

@gopherbot gopherbot commented Jan 3, 2020

Change https://golang.org/cl/213177 mentions this issue: time: remove code duplication

@gopherbot gopherbot closed this in d99fe1f Mar 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

4 participants
You can’t perform that action at this time.