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

Implement timestamp minute counting #154

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Conversation

lbirkert
Copy link

@lbirkert lbirkert commented Aug 9, 2024

Fix #151

This has several benefits:

  • Allows for a more accurate counting of minutes
  • Removes the need for update queries for every session every minute
  • Allows the bot to count minutes even when down

Copy link

@AdamEXu AdamEXu left a comment

Choose a reason for hiding this comment

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

hurry up and approve this code

Copy link

@AdamEXu AdamEXu left a comment

Choose a reason for hiding this comment

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

@techpixel
Copy link
Collaborator

techpixel commented Aug 14, 2024

hey, I can't approve & merge this without thorough testing. I'll send you the necessary creds. I'm testing it locally on my own machine using - I'll let you know if it works or not.

@techpixel techpixel marked this pull request as draft August 14, 2024 01:40
src/clock.ts Outdated Show resolved Hide resolved
Copy link

@AdamEXu AdamEXu left a comment

Choose a reason for hiding this comment

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

i already said i approved, why did you request a review?

@lbirkert lbirkert marked this pull request as ready for review August 16, 2024 15:33
@lbirkert lbirkert requested a review from rutmanz August 16, 2024 15:34
Copy link
Contributor

@rutmanz rutmanz left a comment

Choose a reason for hiding this comment

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

I haven't tested but the code lgtm

src/extensions/slack/index.ts Show resolved Hide resolved
@lbirkert
Copy link
Author

(had to undo the change, as it would be a bug not to, and pixel already said this would be attacked in a future PR)

@techpixel
Copy link
Collaborator

techpixel commented Aug 19, 2024

Tested and the code lgtm. Will merge when I get home. (Merging will require bot downtime)

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.

Using timestamps to record session time instead of a counter
5 participants