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

New Spoon - Hydrate - Drink Water reminder #257

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

luizanao
Copy link

@luizanao luizanao commented Jan 2, 2022

Hydrate

This is a very simple spoon that emits a system pop-up notification every X hours to remind folks to drink water.

Usage:

hs.loadSpoon("Hydrate")
spoon.Hydrate.reminder_interval_minutes = 60
spoon.Hydrate.reminder_days = {"Monday", "Wednesday", "Friday"}
spoon.Hydrate:start()

Source/Hydrate.spoon/init.lua Show resolved Hide resolved
"Sunday"
}

--- Hydrate.Notifier
Copy link
Member

Choose a reason for hiding this comment

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

This API is going to be tricky to express usefully with the way our docstrings work, because Notifier isn't defined anywhere as a module, and in fact it's not a separate module (and if it were, it would get its own page in the docs).

I wonder if it would be possible to restructure things a bit so the API is all in the Hydrate namespace?

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for the advice @cmsj . This is literally my first interaction w/ hammerspoon (and lua). I will take a look in other projects and the docs and come up w/ some solution :)

Copy link
Member

Choose a reason for hiding this comment

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

In theory, if you rebase your branch off our master branch, you should get docstrings errors as comments now. I'm pretty sure I've finally figured out a way to make that work :)

Copy link
Author

Choose a reason for hiding this comment

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

just rebased, lets see :)

Copy link
Author

Choose a reason for hiding this comment

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

I rebased asap I saw your comment @cmsj but I guess the changes weren't in master yet, due to CI delays. I just did it again now. Could you allow CI to run again pls?

@cmsj cmsj marked this pull request as draft January 14, 2022 11:47
@luizanao luizanao marked this pull request as ready for review January 14, 2022 13:47
@luizanao luizanao requested a review from cmsj January 14, 2022 13:47
just a reminder to help you drink water regurlarly
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.

None yet

2 participants