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

Use a namespace for the variables #14

Open
aero31aero opened this issue Nov 26, 2017 · 2 comments
Open

Use a namespace for the variables #14

aero31aero opened this issue Nov 26, 2017 · 2 comments

Comments

@aero31aero
Copy link

Variable names like SOUND etc are quite common to be accidentally set up/conflict with some other tools. Recommend prefixing with NOTIFY_ for ease.

Example: NOTIFY_SOUND, NOTIFY_MIN_INTERVAL etc.

@kaustubhhiware
Copy link
Owner

From what I understand, you suggest it should be possible to change these attributes without editing the script ?

If that is the case, then prefixing NOTIFY_ makes perfect sense.
Now, to achieve this, another approach could be
notifyre --set min_interval 10 or something similar.

@aero31aero
Copy link
Author

Since we are sourcing the scripts and those variables are declared globally in the script, they are now in the global namespace.

Steps to reproduce:

$ cd /path/to/git/repo
$ source ./notifyre.sh
$ sleep 2 # no notification as expected
$ export MIN_INTERVAL=1
$ sleep 2 # notification gets generated

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

No branches or pull requests

2 participants