Add Observer.Recover support to AbstractClientObserver
The patch adds Observer.Recover support to AbstractClientObserver. The param is passed in the new constructor of AbstractClientObserver to avoid breaking API/ABI adding a new virtual method. The old constructor sets the recover flag to false by default, to keep compatibility with older versions.
This is more or less what I had in mind, but: * I don't like the method name recover() - it sounds like it ought to mean "try to recover [something] [from something] and return true on success" rather than just being an accessor. Could it be hasRecover() or willRecover() or something? * Could the old (filter) constructor be implemented as a call to the new (filter, bool) constructor? Do C++ constructors work like that? AbstractClientObserver::AbstractClientObserver(... filter) : AbstractClientObserver(filter, false) {} AbstractClientObserver::AbstractClientObserver(... filter, bool recover) : mPriv(new Private) { mPriv->channelFilter = channelFilter; mPriv->recover = recover; } (If that doesn't work in C++, then never mind.) * It'd be nice to have a convention for noting "what we'll do when we break ABI" - perhaps a comment containing "ABI-break"? - and mark these two constructors as "// ABI-break: combine (filter, bool) and (filter) constructors into (filter, bool=false)", or something
(The first of those points, the name of the recover() method, is the only one I feel strongly about.)
(In reply to comment #2) > This is more or less what I had in mind, but: > > * I don't like the method name recover() - it sounds like it ought to mean "try > to recover [something] [from something] and return true on success" rather than > just being an accessor. Could it be hasRecover() or willRecover() or something? Updated. Changed to shouldRecover as agreed on IRC. > * Could the old (filter) constructor be implemented as a call to the new > (filter, bool) constructor? Do C++ constructors work like that? > > AbstractClientObserver::AbstractClientObserver(... filter) > : AbstractClientObserver(filter, false) > {} > > AbstractClientObserver::AbstractClientObserver(... filter, bool recover) > : mPriv(new Private) > { > mPriv->channelFilter = channelFilter; > mPriv->recover = recover; > } > > (If that doesn't work in C++, then never mind.) Unfortunately that is not possible in C++, but I improved this and moved the members initialization to a new Private class constructor. > * It'd be nice to have a convention for noting "what we'll do when we break > ABI" - perhaps a comment containing "ABI-break"? - and mark these two > constructors as "// ABI-break: combine (filter, bool) and (filter) constructors > into (filter, bool=false)", or something I didn't change that but I agree that should have a convention here, let's discuss this more and then we can do it later.
Better. Ship it!
Merged upstream. It will be in next release 0.3.2
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.