Skip to content

Conversation

@Simon-Laux
Copy link
Contributor

@Simon-Laux Simon-Laux commented Dec 10, 2024

This has the advantage that fields are named/labled and there can be potentially more than 2 (data1 & data2) fields on the events.

This removes the need for the (potentialy confusing) overloading data1 with dc_event_get_data1_str to add more information to events.
And also allows gradual/partial moving to the jsonrpc api, when needed or wanted.

Context/intention: seems logical and was easy to add, no personal grand plan behind it, just wanted to make life easier for UI devs.

Disclaimer: haven't tested it yet, but should work as it is really simple.

…tion of an event

This has the advantage that fields are named/labled and there can be potentially more than 2 (data1 & data2) fields on the events.

This removes the need for the (potentialy confusing) overloading data1 with `dc_event_get_data1_str` to add more information to events.
And also allows gradual/partial moving to the jsonrpc api, when needed or wanted.
@Simon-Laux Simon-Laux requested a review from link2xt December 12, 2024 02:10
@Simon-Laux
Copy link
Contributor Author

Simon-Laux commented Jan 8, 2025

needs to be conditional on the jsonrpc feature flag. But I also ask myself if we still need the jsonrpc feature flag at all?

edit: I made #6417

@hpk42
Copy link
Contributor

hpk42 commented Jan 13, 2025

are there no tests against the jsonrpc API in the core?
Adding such a method without a test that becomes part of CI feels brittle.

@link2xt
Copy link
Collaborator

link2xt commented Jan 13, 2025

We kind of want to deprecate legacy Python bindings at some point, and it is the only part that tests CFFI.

What we really need is some UI to use this API instead of the old API. I'm also fine with merging this as is if it makes experimenting with this easier, if it's broken it is not a problem, but we need iOS or Android or DeltaTouch to use it eventually.

@r10s
Copy link
Contributor

r10s commented Jan 13, 2025

but we need iOS or Android or DeltaTouch to use it eventually

for Android/iOS, there is no need nor plans just now. but on the next event added, it may become handy - so, not sure if it makes sense to merge this PR, usually, we do not do that without UI counterpart. i would be fine to put it to resurrection and revive it, it if gets needed

@Simon-Laux
Copy link
Contributor Author

Ok let's put it to project resurrection, we know it exists and can be used when new events have more than 2 fields.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Closed PRs & Issues

Development

Successfully merging this pull request may close these issues.

6 participants