-
Notifications
You must be signed in to change notification settings - Fork 3
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
Lift errors #4
Comments
Hi @AryanGodara, nice to see that you're keen on contributing to the MIDI project! I think it's better to stay focused on one task before taking on others. Do you prefer this issue or #3 to work on? I think they're similarly involved. #3 requires a little bit of knowledge about music as well. In case you (or anyone else) is worried about issue shortage: I, and possibly @rand00 as well, will create more issues these days. |
Btw, if this issue seems hard to approach to anyone: I recommend reading the Real World OCaml chapter on error handling. And also: don't hesitate to ask if the issue is unclear! |
Oh, @AryanGodara, I've just seen that you've already opened a PR for #3 🎉 So, in that case: yes, feel free to start working on this if it's interesting for you. As mentioned, we'll also open other issues soon for people to have a bigger choice. Sorry for the confusion before! |
No worries @pitag-ha , I'll start work on this first thing tomorrow morning! |
@pitag-ha , I am working on this. |
Ok, noted. No stress! |
@AryanGodara , I've just unassigned you from this issue in case someone else wants to work on it, given that you're working on #13 now. Does that sound good to you? |
Yes, thats perfect, I'll work on the CI issue for now! |
@pitag-ha Can i come back and continue with this issue now, since i've completed the other tasks? |
Hi @AryanGodara, that's up to you! I've just written a longer message about that here. |
I've also just seen that you're working on an issue on |
Yes, I'll complete the fry task first. And then switch over to this one. I just did not want to leave it as it was different from the other tasks (error handling) and I started it earlier 😅 |
We currently have quite an ad-hoc implementation for
cardio-crumble
. In particular, it currently doesn't have a library-binary structure. One of the things we'll need to do for a hygienic structure is to avoid raising wherever we encounter errors. Instead, we should use the OCamlResult.t
data structure in order to lift errors step by step up to the entry modulesSimple_engine
andStat_engine
.One example of that is the termination of the process in
Midi.Device.play
. Instead,Midi.Device.play
should return aResult.t
value, which I suggest promoting via thePlay
module up to theSimple_engine
andStat_engine
modules, where theResult.t
value should be handled by exiting if there's an error.As a first step for this, you could only lift the error from
Midi.Device.play
until thePlay
module and handle it in there already.The text was updated successfully, but these errors were encountered: