solvuu-build update type of `install` arg

Currently, Project.lib takes install : [`No | `Findlib of string], and Project.app takes install : [`No | `Opam]. The `No now seems unnecessary; could instead make this an optional arg. Consider doing this for next release.

Also consider arg types of merlin_file as pointed out in this comment.

Remember to update README.

Mar 15 '17 13:03 agarwal

@agarwal you mean making `No the default? Seems confusing, doesn't it?

Mar 15 '17 21:03 smondet

No, I'm wrong

[install]: Default is [`Findlib name]. This is certainly incorrect for projects containing multiple libs. You should provide a distinct findlib package name for each lib.

?install is already optional, what do you mean with “could instead make this an optional arg”?

Also, the example in the README.md still uses ~pkg:...

Mar 15 '17 22:03 smondet

you mean making `No the default?

No, the default would be semantically the same as it is now. The proposed change would be

Project.lib : ?install:[`Findlib of string] -> ...

instead of the current

Project.lib : ?install:[`No | `Findlib of string] -> ...

the example in the README.md still uses ~pkg:..

Thanks for point this out. Forgot to update README. Maybe I'll hold off since the argument is again being changed.

Mar 16 '17 00:03 agarwal

The current is already optional?

Project.lib : ?install:[`No | `Findlib of string] ->

https://github.com/solvuu/solvuu-build/blob/master/lib/project.mli#L210

You mean then replacing ~install:`No with ?install:None?

Mar 16 '17 00:03 smondet

Sorry, I'm being sloppy. Right, it is already optional. Updated my previous comment. The default would still be `Findlib name. Difference is if you don't want to install, then you currently would do ~install:`No. With the proposed change, you would have to do ?install:None, which is a bit strange. I feel I'm giving too much meaning to None now, but on the other hand right now I feel `No is redundant to None. That's why I opened this issue. I'm not convinced yet what is better.

Mar 16 '17 00:03 agarwal

Is it even possible for a function to behave differently if you don't pass an optional argument vs passing None for said argument? (which, if I understand correctly, would be the case in your proposal).

Anyway, I think the current implementation is fine, but I'd slightly prefer not installing by default (that would be the intuitive behaviour, in my mind).

Apr 05 '17 20:04 copy

Is it even possible for a function to behave differently if you don't pass an optional argument vs passing None for said argument?

Indeed, so it would have to be: Project.lib : ?install:[`Findlib of string] option -> ... and thus you could pass ~install:None to override the default.

I'd slightly prefer not installing by default (that would be the intuitive behaviour, in my mind).

This option doesn't cause anything to get installed, only enables installing. Most libraries are meant to be installed.

Apr 05 '17 21:04 agarwal

Indeed, so it would have to be: Project.lib : ?install:[`Findlib of string] option -> ... and thus you could pass ~install:None to override the default.

I'd rather leave it flat as is than create a mess with ?install:None, ~install:None, ?install:(Some None), ~install:(Some …), etc. (especially where the first two do something different).

Apr 05 '17 21:04 copy

Seems the double options are causing more confusion than helping. I've updated this issue to also reconsider other args as pointed out here.

Apr 07 '17 19:04 agarwal