update type of install arg

[agarwal]

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.

[smondet]

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

[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:...

[agarwal]

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.

[smondet]

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?

[agarwal]

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.

[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? (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).

[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?

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.

[copy]

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).

[agarwal]

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