[pigeon] Update README.MD #2722

keciman · 2022-10-20T18:29:27Z

@gaaclarke I feel like the wording should be better here. It was quite missleading to me and also for other people from what I've seen.
About two things:

That it is synchronous. Which is only on the native side and it makes a person think that it is also sync on the Flutter part
That is is "faster" even tho it is based on MethodChannel.

@gaaclarke

@gaaclarke I feel like the wording should be better here. It was quite missleading to me and also for other people. About two things: 1. That it is synchronous. Which is only on the native side and it makes a person think that it is also sync on the Flutter part 2. That is is "faster" even tho it is based on MethodChannel. I hope this note from me makes sense and will help to make this SDK a better tool.

gaaclarke

I appreciate the sentiment. I think there is room for improvement but these changes aren't doing a better job at informing users.

gaaclarke · 2022-10-20T18:39:24Z

packages/pigeon/README.md

+By default Pigeon will generate synchronous handlers for messages on the native side. 
+The Flutter part is not asynchronous! If you want to be able to respond to 
+a message asynchronously you can use the `@async` annotation as of version 0.1.20.


What is generated on the native side depends on if you have a @HostApi or @FlutterApi.

Using an exclamation isn't really the right tone for the document =)

Thank you for your reply. I totally agree. Just wanted to point it out and I was not sure what is the best way.
Should I try to come up with better wording or can you assign it to somebody to rephrase these two parts ? :)

It's you, I believe in you, you got this. You ran into something that frustrates you in the documentation. You have all the information on how to make this better. Make it better, make it accurate, make it professional and it's in =)

Ok hopefully this version will be more understandable for users :)

P.S.: So glad to start giving back to the community after developing in Flutter for 3 years.

gaaclarke · 2022-10-21T00:14:02Z

packages/pigeon/README.md

@@ -1,7 +1,7 @@
 # Pigeon

 Pigeon is a code generator tool to make communication between Flutter and the
-host platform type-safe, easier and faster.
+host platform type-safe, easier and faster to implement.


While pigeon right now runs at the same speed and platform channels. We are about to land a change that makes them faster. That was always the goal. Once we can abstract away the medium, we can start making improvements.

I understand it is the goal. But the main problem why I want this text to change is I spent one working day implementing pigeon to make my code run faster and after testing if it is faster and how much to realize it is the same speed :( So I want it to be clear for others too.
So should i just delete the whole word "faster" for now?

But I am looking forward for the faster version 💯

Sorry, since this was authored now for the same feature set, pigeon is faster than MethodChannels. I don't think it's misleading to say that it is faster.

(note too that it was always faster than method channels because of one less layer of indirection that results from multiplexing the channels)

"MethodChannels" doesn't appear in the sentence anywhere. Someone could easily assume that the comparison is with message channels, not method channels, and it certainly isn't faster that the thing it is implemented using.

gaaclarke · 2022-10-21T00:19:16Z

packages/pigeon/README.md

+By default Pigeon will generate synchronous handlers for messages. 
+The API can return data synchronously but the request function is always asynchronous.
+
+If you want to be able to respond to a message asynchronously you can use the `@async`


How about:

By default Pigeon will generate synchronous handlers for messages (i.e. the host code for @HostApi's and the Flutter code for @FlutterApi's). Using the @async annotation on a method in the Pigeon file makes the generated methods respond asynchronously instead.

I try to clarifiy something else that does not have to be obvious to people that see the package for the first time.
That is that the other side will be always async no matter what you do. First time I just saw info that it is synchronous and from the context I did not know it is only one sided synchronous code. I understand that from perspective of a person that is doing this often it might look different and there might be obvious reasons for which it wouldnt make sense that it would be both sides sync. But thats what I thought combined with the info that it is also faster. I can rephrase it a bit more so its obvious from your technical point of view and maybe the more naive point of view.

I think this is still a bit confusing because "request function" isn't a term we use. I think there is some confusion in vocabulary between the generated function the client calls and the handler that gets invoked as a result.

I'd refer to the former as "generated methods" and "handlers". So the generated methods are always asynchronous, but the handlers are synchronous by default but can be asynchronous with the @async annotation. I don't think this clarifies.

suggestion:

By default Pigeon will generate synchronous handlers for messages and asynchronous methods. If you want a handler to be able to respond to a message asynchronously you can use the @async annotation as of version 0.1.20.

stuartmorgan-g · 2022-11-15T21:16:05Z

@gaaclarke It looks like this is ready for another round of review/discussion.

stuartmorgan-g · 2022-12-13T21:07:20Z

@gaaclarke Ping on this review, from triage.

stuartmorgan-g · 2023-01-10T21:08:20Z

@gaaclarke Ping on this review again.

gaaclarke · 2023-01-17T18:09:23Z

packages/pigeon/README.md

@@ -1,7 +1,7 @@
 # Pigeon

 Pigeon is a code generator tool to make communication between Flutter and the
-host platform type-safe, easier and faster.
+host platform type-safe, easier and faster to implement.


Sorry, since this was authored now for the same feature set, pigeon is faster than MethodChannels. I don't think it's misleading to say that it is faster.

gaaclarke · 2023-01-17T18:15:10Z

packages/pigeon/README.md

+By default Pigeon will generate synchronous handlers for messages. 
+The API can return data synchronously but the request function is always asynchronous.
+
+If you want to be able to respond to a message asynchronously you can use the `@async`


I think this is still a bit confusing because "request function" isn't a term we use. I think there is some confusion in vocabulary between the generated function the client calls and the handler that gets invoked as a result.

I'd refer to the former as "generated methods" and "handlers". So the generated methods are always asynchronous, but the handlers are synchronous by default but can be asynchronous with the @async annotation. I don't think this clarifies.

gaaclarke · 2023-01-17T18:18:18Z

packages/pigeon/README.md

+By default Pigeon will generate synchronous handlers for messages. 
+The API can return data synchronously but the request function is always asynchronous.
+
+If you want to be able to respond to a message asynchronously you can use the `@async`


suggestion:

By default Pigeon will generate synchronous handlers for messages and asynchronous methods. If you want a handler to be able to respond to a message asynchronously you can use the @async annotation as of version 0.1.20.

gaaclarke · 2023-01-18T16:57:10Z

Thanks for the feedback and working through some iterations. Since this got buried I didn't want to ask you to have to come back to it. I addressed the feedback in #3058. Please take a look. The big confusion was asynchronous methods being an invariant to the @async annotation. I think this clears that up in the simplest way possible.

keciman requested review from tarrinneal and gaaclarke as code owners October 20, 2022 18:29

github-actions bot added the p: pigeon label Oct 20, 2022

gaaclarke requested changes Oct 20, 2022

View reviewed changes

keciman added 2 commits October 21, 2022 01:03

Update README.md

269b51c

Update README.md

05693ee

gaaclarke reviewed Oct 21, 2022

View reviewed changes

stuartmorgan-g requested a review from gaaclarke November 29, 2022 21:13

gaaclarke requested changes Jan 17, 2023

View reviewed changes

gaaclarke mentioned this pull request Jan 17, 2023

[pigeon] updated readme wrt @async #3058

Merged

11 tasks

gaaclarke closed this Jan 18, 2023

[pigeon] Update README.MD #2722

[pigeon] Update README.MD #2722

Uh oh!

Conversation

keciman commented Oct 20, 2022

Uh oh!

gaaclarke left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stuartmorgan-g commented Nov 15, 2022

Uh oh!

stuartmorgan-g commented Dec 13, 2022

Uh oh!

stuartmorgan-g commented Jan 10, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gaaclarke commented Jan 18, 2023

Uh oh!

Uh oh!