Skip to content

Add a promise/future pair to PendingEventItem #767

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

Merged
merged 4 commits into from
Jul 16, 2024

Conversation

KitsuneRal
Copy link
Member

PendingEventItem screams that it has semantics of a future even with its name - now one can obtain a future from it and do useful things in then(), instead of connecting to pendingEvent* signals. The downside - the future carries no information about the pending item index so it seems of limited use in event item models.

Technically very similar to postEvent(), post() takes ownership
guarantees to the type system, and returns a reference to the pending
event item that is produced in the process of posting anyway and has
all the information in a single structure including the transaction id
that is returned now - instead of the sole transaction id which is
frequently only used to find the pending event item first thing.

The template overload of post() also allows to construct an event
on the fly instead of creating an event object upfront (see
the corresponding change in quotest.cpp).

Other post*() methods still return transaction id in order to retain
interoperability with QML. I haven't seen Quaternion or NeoChat actually
using the returned values from post*() calls though. We might
eventually move on to some Q_GADGET wrapper of PendingEventItem so that
post*() methods could still be fully interoperable with QML but provide
more information that a mere transaction id - if needed.
@KitsuneRal KitsuneRal added the enhancement A feature or change request for the library label Jun 29, 2024
Copy link

@KitsuneRal KitsuneRal requested a review from TobiasFella July 2, 2024 07:21
@KitsuneRal
Copy link
Member Author

@TobiasFella what do you think of switching to const PendingEventItem& or a Q_GADGET as a return value for all post*() calls, eventually or even for 0.9?

@TobiasFella
Copy link
Member

@TobiasFella what do you think of switching to const PendingEventItem& or a Q_GADGET as a return value for all post*() calls, eventually or even for 0.9?

should be fine for us

@TobiasFella
Copy link
Member

looks reasonable, will test later

TobiasFella
TobiasFella previously approved these changes Jul 15, 2024
@KitsuneRal KitsuneRal merged commit dab1d6f into dev Jul 16, 2024
6 checks passed
@KitsuneRal KitsuneRal deleted the kitsune/future-for-pending-event-item branch July 16, 2024 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement A feature or change request for the library
Projects
Status: 0.9 - Done
Development

Successfully merging this pull request may close these issues.

2 participants