rand00 / conntest

MirageOS unikernel to test networking
GNU Lesser General Public License v2.1
16 stars 7 forks source link

Join the `Udp_flow.partial_flow` and `Udp_flow.t` types #3

Open rand00 opened 1 year ago

rand00 commented 1 year ago

These two types became almost the same type over time, where the only difference is that the type t has the extra field feeder.

I think the best solution will be to make the feeder field into an 'a option, and then cancel only if it's Some feeder.

haanhvu commented 1 year ago

Hi, I have finished first steps in mirage/mirage#1402. I'll work on this now.

rand00 commented 1 year ago

Hi @haanhvu - cool!

haanhvu commented 1 year ago

@rand00 Hi, for now I have 2 questions:

  1. What is the name of the new type? Since we merging two types, I'm thinking of the merged name Udp_flow.partial_flow_t?
  2. I have compiled conntest successfully on main branch. However I use VSCode and wonder if there's a VSCode extension that checks compilation errors in real time? I actually installed OCaml Platform. But I didn't see it check compilation errors (don't know if there're further steps to do to use this extension for compilation checking).

Update: I installed ocaml-lsp-server and syntax errors are checked now. So there's only question1 left^^

haanhvu commented 1 year ago

@rand00 Also, now I have another question: To make the feeder field into an 'a option as you said, do we just need to code feeder : 'a option? I did this and it gives me The type variable 'a is unbound in this type declaration error. Sorry very new to Ocaml syntax!

rand00 commented 1 year ago

What is the name of the new type?

As the partial_flow type was named because it was a subset of t, the resulting name should be t. In OCaml t is the convention for the name of the primary type of a module - in this case Udp_flow.t.

Good that you got the realtime typechecking to work (:

haanhvu commented 1 year ago

What is the name of the new type?

As the partial_flow type was named because it was a subset of t, the resulting name should be t. In OCaml t is the convention for the name of the primary type of a module - in this case Udp_flow.t.

Good that you got the realtime typechecking to work (:

@rand00 Thanks! Could you answer the other question I asked here too?

@rand00 Also, now I have another question: To make the feeder field into an 'a option as you said, do we just need to code feeder : 'a option? I did this and it gives me The type variable 'a is unbound in this type declaration error. Sorry very new to Ocaml syntax!

rand00 commented 1 year ago

@rand00 Also, now I have another question: To make the feeder field into an 'a option as you said, do we just need to code feeder : 'a option? I did this and it gives me The type variable 'a is unbound in this type declaration error.

The feeder already has a static type, so there is no need to make it a type-parameter on t. (In case you wanted to, you would write type 'a t = ...). In this way the type of the feeder field would become unit Lwt.t option

haanhvu commented 1 year ago

The feeder already has a static type, so there is no need to make it a type-parameter on t. (In case you wanted to, you would write type 'a t = ...). In this way the type of the feeder field would become unit Lwt.t option

@rand00 Thanks. I changed feeder field type as you said. So in the current code:

let flow = {
              ...
              feeder;
            }

What should feeder be changed to to compile with the new type?

Update: I just studied briefly about Ocaml option and it seems like I have to change feeder to feeder = Some feeder? Don't know if it's true or not but the type error's gone away after the change^^

rand00 commented 1 year ago

I think when the questions become more code-specific, the discussion should move into a specific pull-request. If you have any questions around this just ask. Just prefix the PR with "WIP: ..." when it's not done yet

Update: I just studied briefly about Ocaml option and it seems like I have to change feeder to feeder = Some feeder? Don't know if it's true or not but the type error's gone away after the change^^

Indeed (: The OCaml compiler is very helpful in stating what is wrong - so try to fix all the type-errors from what it complains about - then it's often the case that the resulting code is correct

rand00 commented 1 year ago

Hey @haanhvu - are you having progress?

haanhvu commented 1 year ago

@rand00 I have been having exams since last week. I'll continue this this weekend.

Khadija411 commented 1 year ago

Hi @rand00 If this issue is not solved I would like to work on it.

haanhvu commented 1 year ago

Hi @rand00 If this issue is not solved I would like to work on it.

Yes it's not resolved. Feel free to take it up.