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

Parametrise REVERT_INTERVAL. Cosmetic changes #20

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Parametrise REVERT_INTERVAL. Cosmetic changes #20

wants to merge 1 commit into from

Conversation

alienmind
Copy link

No description provided.

@voghDev
Copy link
Owner

voghDev commented Oct 14, 2017

Hi! Thanks for the PR.
It looks pretty good, I would remark just a couple of things

  • I would not extract the number 8 to a REVERT_INTERVAL variable as it will never change. Facts already happened and the exact time was 8 seconds; I don't think we are going to rewrite the past :-P so there's no need on a variable. I think 1..8 looks more readable than 1..$VAR.
  • I think it is funnier to remove the -n option in the echo "." line, so each "." is painted in a different line, and the effect is funnier for the reader.

Let me know what you think

@aqreed
Copy link

aqreed commented Oct 14, 2017 via email

@voghDev
Copy link
Owner

voghDev commented Oct 15, 2017

[Update] Still trying to apply your PRs, but for some reason the for loop iterates only once, instead of 8. Only way to make it work is current code: 1 2 3 4 5 6 7 8

I'll keep on trying and let you guys know :)

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