Hi Romain, I'm relieved that we came to an agreement, I'm much more comfortable with the new patch. To me the deal breaker was not the mutex, but the possibility for the user to have two versions of a stream and only insert in one. An imperative / in-place insert_metadata would not have allowed this, unless we create a copy operator...
About your patch: I don't think the mutex is really needed in the insert_metadata operator: the only concurrent executions are on the writing side (set a new metadata) not on the reading side (read the metadata and put it in the stream), so it seems that concurrency has no real bad effect (still, it's not a bad idea to protect against it). I think it would be simpler to put the registration of the operator in insert_metadata.ml, not lang_builtins.ml. Also, we don't need of_fst/snd_product_t since we already have Lang.to_product. In the documentation, you should write "of type (metadata)->unit" to stick with our syntax of types. Perhaps replace metadata with [(string*string)] or say what it means. Also I would keep the old category, Track_processing. Other than that, the registration of the new operator looks clean to me. I did not test it but read it carefully. Nice job, you even put back the optional "id" argument. Concerning the TODO on fresh, we'll take care of that later, but I don't see a solution so it might become the official way to do; it might be a necessary tricky thing. Cheers, -- David ------------------------------------------------------------------------------ Increase Visibility of Your 3D Game App & Earn a Chance To Win $500! Tap into the largest installed PC base & get more eyes on your game by optimizing for Intel(R) Graphics Technology. Get started today with the Intel(R) Software Partner Program. Five $500 cash prizes are up for grabs. http://p.sf.net/sfu/intelisp-dev2dev _______________________________________________ Savonet-devl mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/savonet-devl
