Result.{result => t}

[nojb]

Using Result.result instead of Result.t makes it impossible to use the Stdlib Result when vendoring this package (eg when building dune on Windows), which is a headache.

I only took a quick glance at the compatibility package https://github.com/janestreet/result, but it seems to define Result.t as well as Result.result so it looks like doing this renaming would leave everyone happy :)

[ghost]

I just gave this a try, but it seems to break 4.02 compat :(

If it makes packaging difficult, it seems acceptable to remove this definition and add the Result. prefix everywhere.

[rgrinberg]

Or, we could just forget about this headache once and for all and drop 4.02.

@voodoos what do you say? :)

[voodoos]

I think we should wait for Merlin's support of old versions of the compiler to be "frozen" and splitted from the latest branch. If it's urgent I guess we could restrict the version of csexp required by merlin only when building on 4.02 ?

[voodoos]

(Sorry, misclicked on the close button.)

@trefis ?

[trefis]

I just gave this a try, but it seems to break 4.02 compat :(

I think that's because Result.t is defined here as an abstract type. If it reexported the constructors, then I believe the build would succeed.

If it's urgent I guess we could restrict the version of csexp required by merlin only when building on 4.02 ?

No need, opam would prevent you from using the new version of csexp on 4.02 anyway.

But I think fixing result is the better choice here.

[ghost]

I'm guessing this is no longer relevant after #17.

[nojb]

Thanks!