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

Fix #63: The week_start is different from WakaTime #64

Merged
merged 1 commit into from
Jun 23, 2021
Merged

Fix #63: The week_start is different from WakaTime #64

merged 1 commit into from
Jun 23, 2021

Conversation

ryul1206
Copy link
Contributor

Thank you very much for allowing PR.
This PR is to the master branch since I couldn't find any development branch.

Copy link
Collaborator

@yozachar yozachar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR!

@yozachar yozachar requested a review from athul June 23, 2021 02:17
@athul
Copy link
Owner

athul commented Jun 23, 2021

Hey, could you explain what this PR actually does? I'm a tad confused 😅

@ryul1206
Copy link
Contributor Author

Hello @athul ,
this action uses one-week information via last_7_days.
In the current code, there is 7 days difference between week_start and week_end.

week_start = week_end - datetime.timedelta(days=7)

However, I think the interval between the start and end of a week is only 6 days.
[mon]-1-[tue]-2-[wed]-3-[thu]-4-[fri]-5-[sat]-6-[sun]

So, for example, Monday is sunday - datetime.timedelta(days=6). As I mentioned in #63, my Wakatime information and the current waka-readme title are not the same, so I suggest this PR.

@yozachar
Copy link
Collaborator

@athul, may I add #33 asks about a bi-weekly status, if we allow timedelta to be changed by workflow configuration, will we be able to get statistics within a custom time slice?

@athul
Copy link
Owner

athul commented Jun 23, 2021

@ryul1206 That makes sense 👍 . Thanks for the Response. @joe733 As mentioned in #33 The API doesn't support it 😞

If everything fine, I'll move to merging this

Copy link
Owner

@athul athul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@yozachar
Copy link
Collaborator

yup, good to go 👍🏻

@yozachar yozachar merged commit 1104abb into athul:master Jun 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants