-
Notifications
You must be signed in to change notification settings - Fork 94
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
Update Lwt tutorial examples to match new tutorial #137
Conversation
In particular: - Change `C.log` to `log_s`. `log` is the emergency non-blocking version that discards data if the buffer is full. We should not encourage its use in a tutorial. Also, it doesn't demonstrate using Lwt. - Use `>>=` rather than `>|=` because the tutorial didn't introduce it yet. - Don't use `_` to ignore exceptions in timeout example. - Moved some functions out of `start` for clarity. - Remove mvar and stream examples, as they're no longer in the tutorial.
(I feel a bit bad about deleting the mvar and stream examples. @mor1 : do you think we should keep these?) |
Guess it depends whether they're useful :) |
I try to stay away from mvars and streams (and I usually don't use them -- apart stream in some special cases). So I'm happy with that change. |
I don't use mvars at all and hardly ever use streams (iirc they're hard to use correctly). The main thing I use is "lwt.task" which is like an immutable mvar (aka an Ivar or deferred or thread) |
I use streams over here in IKEv2... and think this is a viable use case (merging the different streams, data from kernel, data via udp, timer tick)... mvar are also useful synchronisation primitives (I do use them in jackline, but am not yet satisfied with their usage over there)... but I guess it doesn't need to be in the lwt tutorial (though some people told me that mirage-skeleton is too basic, examples like a multi-client echo server (including broadcasting) would be useful) |
Ok so perhaps the things to do are: leave them in skeleton for now (assuming they build) and maybe file an issue against |
..or just remove them and at some point add some with a suitable story and examples? |
Yes that's the other obvious option; but as I said, I generally prefer to keep working examples around as they might be useful to someone and I don't think having them in there without reference is particularly confusing. Ofc, if there's a good source of more generic Lwt examples showing how to use some of these features, then there's no need for them to remain here particularly. But if there isn't another alternative, why reduce the number of viable code samples out there? |
I don't use mvars much either. Could you explain where they're useful? The main place I've seen them used is in the tcp stack, but several people have suggested the code would be clearer if we removed them. I guess they're more useful in a multi-core preemptive system where you want a consumer and producer to run in parallel on different cores, but in Lwt that doesn't apply: in many cases (as in the example code) you might as well have the producer call the consumer directly. e.g. the example does
But if you dropped the mvars, you could replace the whole thing with just:
|
@talex5 I don't know why If Similarly, if actually |
this should be merged since the website part is online now. objections? |
As noted above -- I'd probably prefer to see the mvar and stream examples not be removed in this PR. But either way, it should be merged soon, yes. |
This is a bit of a hijack, but if any of the participants here has interest in/complaints about Lwt's streams, I would really appreciate if you can comment here: Lwt_stream sucks (?) (ocsigen/lwt#250). Perhaps the situation can be improved in some way. |
I vaguely remember @pqwy making me a funny account of |
Well no, remove things that are unused, they are only distractions to newcomers. That's the kind of way you end up with useless crap on the mirageos website... |
In particular:
C.log
tolog_s
.log
is the emergency non-blocking versionthat discards data if the buffer is full. We should not encourage its
use in a tutorial. Also, it doesn't demonstrate using Lwt.
>>=
rather than>|=
because the tutorial didn't introduce ityet.
_
to ignore exceptions in timeout example.start
for clarity.See: mirage/mirage-www#458