-
-
Notifications
You must be signed in to change notification settings - Fork 97
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
remove unused dont_notify
and coalesce
push actions
#643
remove unused dont_notify
and coalesce
push actions
#643
Comments
also in the cleaning-up-pushrules arena: #637 |
for reference: these actions both date back to matrix-org/matrix-spec-proposals#10. I think they are relics of an early draft which never got cleaned up. |
|
the rationale for |
given we gatekeep the spec and require the use of namespaces for custom implementations, do we need to reserve the keyword? |
Unless I'm misunderstanding how the rules work - in which case, I would be delighted to be corrected - it is absolutely a no-op. Including it in the list has absolutely no effect. It's even valid to have a push rule with both Now, there may be an argument for having a specified no-op, but: it is a no-op.
That is exactly my point, and I contend that its very existence is a source of confusion, since it leads to the questions of "when do I need to set it? what happens if I don't set it? Is it a no-op or not?"
Well, I wouldn't propose forbidding its use: just marking it deprecated.
I think it would be less of a bad thing if clients had do to it: which is to say that for a rule to be valid, it has to either specify At the very least, the spec should make clear that it is a no-op. But I think we can go further. |
dont_notify is used in matrix-org/matrix-spec-proposals#2153. |
yes, but not in a way that makes any material difference here. |
dont_notify
and coalesce
push actionsdont_notify
and coalesce
push actions
cf: https://matrix.org/docs/spec/client_server/r0.6.1#actions.
dont_notify
is a no-op and its presence causes confusion.coalesce
has never been implemented afaikThe text was updated successfully, but these errors were encountered: