Get the number of notifications#793
Conversation
tsipinakis
left a comment
tsipinakis
left a comment
There was a problem hiding this comment.
Some naming nitpicks. Otherwise LGTM 👍
dunstctl
Outdated
| property_set paused variant:boolean:"$2" | ||
| fi | ||
| ;; | ||
| "status") |
There was a problem hiding this comment.
status is a bit ambiguous, one could assume this is paused status. I'd suggest count as the command (unless you have a better idea?)
There was a problem hiding this comment.
I have no better idea as count. I'll change it…
src/dbus.c
Outdated
|
|
||
| " <property name=\"displayed\" type=\"u\" access=\"read\" />" | ||
| " <property name=\"history\" type=\"u\" access=\"read\" />" | ||
| " <property name=\"waiting\" type=\"u\" access=\"read\" />" |
There was a problem hiding this comment.
Similar here, I'd name it displayedLength to avoid naming conflicts for future expansions.
There was a problem hiding this comment.
I would name them all in the same way:
displayedLengthhistoryLengthwaitingLength
Without the technical knowledge, it's hard to get, what those properties shall return. Maybe we can find names?
There was a problem hiding this comment.
I gave an example for displayed but I did mean to change all of them :). Your list of names looks good to me.
Codecov Report
@@ Coverage Diff @@
## master #793 +/- ##
==========================================
- Coverage 65.02% 64.91% -0.12%
==========================================
Files 30 30
Lines 5275 5284 +9
==========================================
Hits 3430 3430
- Misses 1845 1854 +9
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
Shall I provide unit tests too? As I can see there are no tests for the dbus interface or |
|
Yeah that'd be great. More testing is always better |
tsipinakis
left a comment
tsipinakis
left a comment
There was a problem hiding this comment.
LGTM, as @fwsmit said, if you can also add some tests for this it'd be great but I'm happy to merge as-is if you don't have the time.
|
I guess that it makes sense to merge it now. I won't find time to write tests for the next days or even weeks. |
4 participants
This will add the dbus properties
displayedwaitinghistoryand extends
dunstctlwith the sub-commandstatus, which provides this information.Currently I'm not sure, if
historyis the right term. Maybeshownfits here better.See also the feature request #792.