conntest icon indicating copy to clipboard operation
conntest copied to clipboard

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

Open rand00 opened this issue 3 years ago • 13 comments

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.

rand00 avatar Mar 08 '23 18:03 rand00

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

haanhvu avatar Mar 09 '23 17:03 haanhvu

Hi @haanhvu - cool!

rand00 avatar Mar 10 '23 10:03 rand00

@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 avatar Mar 10 '23 14:03 haanhvu

@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!

haanhvu avatar Mar 10 '23 16:03 haanhvu

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 avatar Mar 13 '23 19:03 rand00

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!

haanhvu avatar Mar 14 '23 08:03 haanhvu

@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

rand00 avatar Mar 14 '23 09:03 rand00

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^^

haanhvu avatar Mar 14 '23 20:03 haanhvu

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 avatar Mar 15 '23 11:03 rand00

Hey @haanhvu - are you having progress?

rand00 avatar Mar 23 '23 07:03 rand00

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

haanhvu avatar Mar 23 '23 07:03 haanhvu

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

Khadija411 avatar Apr 03 '23 02:04 Khadija411

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.

haanhvu avatar Apr 03 '23 04:04 haanhvu