search

matterhorn-chat / matterhorn

A feature-rich Unix terminal client for the Mattermost chat system
BSD 3-Clause "New" or "Revised" License
1.05k stars 77 forks source link

notifyV2: shellcheck pass, add appname & icon #839

Closed fictitiousexistence closed 1 week ago

fictitiousexistence commented 1 week ago

Update echo to printf Pass shellcheck Quote unquoted variables Use $(...) notation instead of legacy backticks Use foo () instead of function foo Add appname and icon options for notify-send

jtdaugherty commented 1 week ago

Thanks for this patch!

For future contributions, something to keep in mind is that these changes need to be committed with logical separation. For example, the change to set the application name and introduce the icon argument are new features that are separate from linting/cleanup introduced by some of the other changes. Having them all in one commit makes it more difficult to selectively revert some of them if there's a problem for some reason, and in general it makes commits more difficult to review.

jtdaugherty commented 1 week ago

(That sort of thing can usually be done very easily by using git add -p rather than git add.)