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

Refactorings #18

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

Refactorings #18

wants to merge 22 commits into from

Conversation

thomas-gruen
Copy link

Hello Igor,

as requested in #17 (comment) :-)

Here are my refactorings. I hope you'll find them useful. A word of caution: I have only 'tested' the program directly on my watch, since I couldn't get the simulator to run. So a thorough test is definitely advised.

The refactorings contained in this pull request were a prerequisite for my alterations of the program (see https://github.com/thomas-gruen/garmodoro/issues?q= ). I suspect, you won't incorporate these into your code, since they modify the programs behavior. However, if you like to, feel free to pull or cherry pick from my repo.

Last but not least, I want to thank you for writing the program in the first place and for making it available for free so graciously. It was a lot easier to modify this program than to start from scratch.

Best regards,
Tom

 - created a module that will act as a namespace for all
   pomodoro related functionality
added plan for next refactorings in TODOs
fixed centerY bug in GarmodoroView
all TODOs finished
cached tickStrength and tickDuration to reduce battery strain
bugfix: variable height should be dc.getHeight()
 - moved properties.mk to properties.mk.example
 - added properties.mk to .gitignore
 - added section in README.md

Rationale: a new developer will have to change properties.mk .
So an example file (properties.mk.example) is helpful, but
the actual properties.mk file should not be checked into
version control.
@klimeryk
Copy link
Owner

Thank you for creating this PR, @thomas-gruen. I appreciate it - I'll try to have a look through your improvements this weekend and see what can be incorporated into the vanilla version. It might also give me a nudge to go try to refresh some code, so thank you! I'm glad you found it useful for a base for your own version - open-source FTW! 🎉

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.

2 participants