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

Add Pomodoro timer #177

Closed
wants to merge 2 commits into from
Closed

Add Pomodoro timer #177

wants to merge 2 commits into from

Conversation

koozz
Copy link

@koozz koozz commented Jan 25, 2016

A minimalistic Pomodoro timer.

POMODORO="🍅"
#POMODORO="\xF0\x9F\x8D\x85" # echo -e $POMODORO
POMODORO_SECONDS=1500
POMODORO_DATA="$(dirname $0)/pomodoro.data"
Copy link
Collaborator

Choose a reason for hiding this comment

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

The $0 needs to be quoted otherwise if the path contains spaces then things could go wrong.

@keithamus
Copy link
Collaborator

Hey @koozz awesome plugin! I've made some notes about small changes - mostly syntactical, by using shellcheck to lint the file. If you could please correct these then I'll happily merge 😄

@koozz
Copy link
Author

koozz commented Jan 25, 2016

There are 3 messages from shellcheck that I cannot seem to fix.

In pomodoro.1s.sh line 20:
source "$POMODORO_DATA"
^-- SC1090: Can't follow non-constant source. Use a directive to specify location.


In pomodoro.1s.sh line 29:
    local now=$(date +"%s")
          ^-- SC2155: Declare and assign separately to avoid masking return values.


In pomodoro.1s.sh line 45:
  local now=$(date +"%s")
        ^-- SC2155: Declare and assign separately to avoid masking return values.

Is that urgent enough to fix?

@keithamus
Copy link
Collaborator

SC2155 just means you should declare first, then assign after. For example:

local now;
now=$(date +"%s")

As for SC1090, you just need to comment the line above; so:

# shellcheck source=~/.bitbar/plugins/pomodoro.data
source "$POMODORO_DATA"

Or alternatively you can disable the check:

# shellcheck disable=SC1090
source "$POMODORO_DATA"

@matryer
Copy link
Owner

matryer commented Jan 25, 2016

see #180

@keithamus
Copy link
Collaborator

Hey @koozz, thanks very much for working on this - and thanks also for being responsive to my comments and making the right fixes.

Another user (@gorangajic) has also made a Pomodoro plugin - around the same time as yours. We've gone with theirs, and so I'm going to close this one. Please don't take this as a sign that we don't value your work - we really do! Please don't let this put you off from making PRs in the future! We'd love to see some more great code from you 😄.

@keithamus keithamus closed this Jan 25, 2016
@matryer
Copy link
Owner

matryer commented Jan 25, 2016

Here, here.

@koozz
Copy link
Author

koozz commented Jan 26, 2016

Sorry, I didn't know about the other work in progress.
Just fiddled around and made this one. It's pretty minimalistic, but I'll keep it around for my own use :)

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

3 participants