doc: Express what the comment says #50
Conversation
| //! assert_eq!(subscriber1.next().await, Some("D".to_owned())); | ||
| //! assert_eq!(subscriber2.next().await, Some("D".to_owned())); |
There was a problem hiding this comment.
Hm, both the previous and new code expect there to be two updates? That makes no sense, the second .await should wait forever... But this is a doctest that actually runs as part of the test suite, right?
There was a problem hiding this comment.
I was surprised too, but I've tried outside the doctest and it works. I don't know if the doctest is run by the CI.
There was a problem hiding this comment.
Anyways, can you remove the second assert? We can figure out why it's working later, the example should not rely on it working IMO.
There was a problem hiding this comment.
Oh wait. It was calling next().await from subscriber1 and from subscriber2, it's not the same subscriber!
There was a problem hiding this comment.
I still believe the modification is correct. It was error-prone; proof is we have bugged on this together 😛.
There was a problem hiding this comment.
Oh I see now. Well actually then the original code was correct, it showed that both a previously up-to-date subscriber and a lagged subscriber receive the same value. Not sure if this PR actually makes sense 😅
The comment says: “If multiple updates have happened without the subscriber being polled, the next poll will skip all but the latest.”, but the code shows a single `Observable::set` followed by two `Subscriber::next`. I would personally expect the opposite. This patch updates the example to get 2 `Observable::set` and 1 `Subscriber::next`. The rest of the example has been updated to increase the illustration letter.
|
This was not approved, I reverted via force-push and will likely raise another PR to improve the example. |
The comment says: “If multiple updates have happened without the subscriber being polled, the next poll will skip all but the latest.”, but the code shows a single
Observable::setfollowed by twoSubscriber::next. I would personally expect the opposite.This patch updates the example to get 2
Observable::setand 1Subscriber::next. The rest of the example has been updated to increase the illustration letter.