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

weekOfYear week does not count the day of the week correctly for 'ru' locale #1371

Closed
ushakov-igor opened this issue Feb 3, 2021 · 6 comments
Closed

weekOfYear week does not count the day of the week correctly for 'ru' locale #1371

ushakov-igor opened this issue Feb 3, 2021 · 6 comments
Labels
☢️Bug Something isn't working good first issue Good for newcomers

Comments

@ushakov-igor
Copy link

ushakov-igor commented Feb 3, 2021 •
edited
Loading

Describe the bug
I have an issue with the week(). Maybe similar problem was there #956

Expected behavior
I tried with date '2010-12-30' and it's return 1 week. But I think it should be 52.

Information

  • Day.js Version [1.10.4]
  • OS: [macOS Big Sur 11.1 ]
  • Browser [Chrome 88]
  • Time zone: [GMT+3]
@ushakov-igor ushakov-igor changed the title weekOfYear week method doesn't seems to handle week number 53 for 'ru' locale weekOfYear week does not count the day of the week correctly for 'ru' locale Feb 3, 2021
@iamkun
Copy link
Owner

iamkun commented Feb 3, 2021

Yes, we should set https://github.com/iamkun/dayjs/blob/dev/src/locale/ru.js to yearStart: 4

Would you like to submit a PR to fix it?

@iamkun iamkun added good first issue Good for newcomers ☢️Bug Something isn't working labels Feb 3, 2021
@KashyapPavra
Copy link

@iamkun if the bug is unassigned I would love to contribute

@iamkun
Copy link
Owner

iamkun commented Feb 3, 2021

go ahead please

@tlegen-k
Copy link
Contributor

tlegen-k commented Feb 3, 2021

I am new to committing. And would like to follow up on the issue.
Just submitted request for merge. And is it had no conflicts was able to merge it with original branch.
Have i done everything correctly?

@tlegen-k tlegen-k mentioned this issue Feb 3, 2021
Merged
@ushakov-igor
Copy link
Author

I think everything is good. Maybe @iamkun can merge and up version?🙏

@iamkun
Copy link
Owner

iamkun commented Feb 4, 2021

fixed in #1372

@iamkun iamkun closed this as completed Feb 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
☢️Bug Something isn't working good first issue Good for newcomers
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@iamkun @ushakov-igor @KashyapPavra @tlegen-k