Bug 91755

Created attachment 117914 [details] [review]
the actual patch

Comment on attachment 117913 [details] [review]
preparation: fix whitespace in the spec XML

If we had always had a policy of no trailing whitespace (and no tabs), I'd certainly be fine with applying that policy like this.

However, we deliberately avoid fixing whitespace that is not in or adjacent to the lines you're touching for some other reason, so that patches can backport more easily.

I think this particular change is OK because we never backport the spec anyway (what would that even mean?), but please don't do the same to anything else in the tree.

Comment on attachment 117914 [details] [review]
the actual patch

Review of attachment 117914 [details] [review]:
-----------------------------------------------------------------

This looks good, and I like the new name better, but it shouldn't land until the reference implementation understands arg0has.

I'm about to release 1.10.0; we can perhaps have this in 1.11.0.

Is it really necessary to add this to the reference implementation? 

For us (systemd upstream) this would be no priority, since we only need this for the udev usecase, but that's nothing we'll do on dbus1, but only on kdbus. Hence we are fine if this is implemented in sd-bus/kdbus only for now, as long as it is in the spec.

Also, I think dbus should enforce a stricter whitespace regime and refuse unclean commits, and clean up the existing codebase once. While this will making bakports harder across the cleanup boundary, later on it will makes things much easier!

But anyway, this is of course a separate issue, and the only reason I added this here as second patch is that it annoyed me so much when I added the spec extension, and I thought it might be time to start the clean-up at least on the more auxiliary parts of the codebase.

(In reply to Lennart Poettering from comment #4)
> Is it really necessary to add this to the reference implementation? 

Well, yes. Reference implementations should support as much of the protocol/API as is technically possible, otherwise they make a poor reference :-)

Created attachment 119446 [details] [review]
signals: factor out match_rule_matches_string_arg()

Created attachment 119447 [details] [review]
signals: add support for arg0has keyword

(In reply to Lennart Poettering from comment #4)
> For us (systemd upstream) this would be no priority, since we only need this
> for the udev usecase, but that's nothing we'll do on dbus1, but only on
> kdbus.

Is this still the case at this point? Do you have other use-cases for this feature?

Looking for "tags" on messages seems generically useful, and it isn't actually very much code, so I wouldn't necessarily object to merging it for 1.11 even if you aren't going to use it any time soon.

ssh://people.freedesktop.org/~smcv/dbus arg0has-91755
http://cgit.freedesktop.org/~smcv/dbus/log/?h=arg0has-91755

Comment on attachment 117913 [details] [review]
preparation: fix whitespace in the spec XML

(applied)

If this is something that you want, or that anyone wants, I'm still waiting for review on the implementation.

-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/dbus/dbus/issues/130.