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 configuration options for notification urgency #49

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

Conversation

niraami
Copy link

@niraami niraami commented Nov 5, 2023

Builds upon and replaces #24

Gives the user access to 2 configuration options that affect what urgency is used for the notification.

  • AUTO_NOTIFY_URGENCY_ON_SUCCESS
  • AUTO_NOTIFY_URGENCY_ON_ERROR

I've also added a test case for the ON_SUCCESS urgency option. I was unfortunately not able to figure out how to add a test case for the ON_ERROR option without modifying unrelated parts of the code quite heavily.
I've also added a section in the README, but I'm unfamiliar with the formatting, and my VS Code extension is not rendering it properly, so I just copied the style and hope it works :)

@niraami
Copy link
Author

niraami commented Jan 5, 2024

Thoughts? I've been using my fork for a while now, seems fine.
@MichaelAquilina

@niraami niraami force-pushed the configurable-notification-urgency branch from e81d562 to 7a2adb1 Compare September 28, 2024 20:58
@niraami
Copy link
Author

niraami commented Nov 30, 2024

Been a year, rebased the changes 2 months ago so they no longer conflict. If you don't like the change just close the PR so I don't have to care about this anymore.

@MichaelAquilina
Copy link
Owner

Hi @niraami sorry for the radio silence but I've had less time to maintain this repo dur to life commitments. The changes look good in principle - I just need to find some time to review and test them. I'll probably have some time during the holiday period.

@niraami
Copy link
Author

niraami commented Dec 6, 2024

No worries, glad to hear it nevertheless. I've been using my fork on all of my machines since I made the PR, so I don't think I can test it any more than I already have :P

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