Skip to content

Remove variants#3491

Merged
rgrinberg merged 6 commits intoocaml:masterfrom
rgrinberg:remove-variants
May 29, 2020
Merged

Remove variants#3491
rgrinberg merged 6 commits intoocaml:masterfrom
rgrinberg:remove-variants

Conversation

@rgrinberg
Copy link
Copy Markdown
Member

@rgrinberg rgrinberg commented May 14, 2020

The experimental variants feature is removed in 2.6. In particular:

  • default implementations are a standard part of the language
  • (using library_variants) is a nice error message
  • Fix the semantics of default_implementation. virtual library and default implementation must exist in the same package.

@jeremiedimino do we need nice error messages when users use write (variant ..) fields? Or is it enough to say that the feature was deleted on the (using ..) declaration?

@rgrinberg rgrinberg requested a review from a user May 14, 2020 22:49
@rgrinberg rgrinberg requested review from emillon and nojb as code owners May 14, 2020 22:49
@rgrinberg rgrinberg force-pushed the remove-variants branch 2 times, most recently from 11ae16a to 91214f7 Compare May 15, 2020 02:59
@ghost
Copy link
Copy Markdown

ghost commented May 19, 2020

Let's go for the second option. We don't need to go out of our way for experimental features. In such cases, the code matters more.

Copy link
Copy Markdown

@ghost ghost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, I like PRs with a lot of read :)

@rgrinberg rgrinberg requested a review from ejgallego as a code owner May 21, 2020 00:08
@rgrinberg rgrinberg added this to the 2.6 milestone May 23, 2020
doc/variants.rst Outdated

This feature is also guarded by ``(using library_variants ...)``.
This feature is guarded by a different language declaration depending on the
version of dune that you're using.
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These side notes should be moved to the bottom of the section

Copy link
Copy Markdown

@ghost ghost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just need some doc tweak

The experimental variants feature is removed in 2.6. In particular:

* default implementations are a standard part of the language
* (using library_variants) is an error

Signed-off-by: Rudi Grinberg <me@rgrinberg.com>
rgrinberg added 5 commits May 28, 2020 17:50
Signed-off-by: Rudi Grinberg <me@rgrinberg.com>
Signed-off-by: Rudi Grinberg <me@rgrinberg.com>
Make sure that default implementations exist in the same package as
their virtual libraries

Signed-off-by: Rudi Grinberg <me@rgrinberg.com>
Signed-off-by: Rudi Grinberg <me@rgrinberg.com>
Signed-off-by: Rudi Grinberg <me@rgrinberg.com>
@rgrinberg rgrinberg merged commit df70a93 into ocaml:master May 29, 2020
rgrinberg added a commit to rgrinberg/opam-repository that referenced this pull request Jun 6, 2020
…lugin, dune-private-libs and dune-glob (2.6.0)

CHANGES:

- Fix a bug where valid lib names in `dune init exec --libs=lib1,lib2`
  results in an error. (ocaml/dune#3444, fix ocaml/dune#3443, @bikallem)

- Add and `enabled_ if` field to the `install` stanza. Enforce the same variable
  restrictions for `enabled_if` fields in the `executable` and `install` stanzas
  than in the `library` stanza. When using dune lang < 2.6, the usage of
  forbidden variables in executables stanzas with only trigger a warning to
  maintain compatibility. (ocaml/dune#3408 and ocaml/dune#3496, fixes ocaml/dune#3354, @voodoos)

- Insert a constraint one the version of dune when the user explicitly
  specify the dependency on dune in the `dune-project` file (ocaml/dune#3434 ,
  fixes ocaml/dune#3427, @diml)

- Generate correct META files for sub-libraries (of the form `lib.foo`) that
  contain .js runtime files. (ocaml/dune#3445, @hhugo)

- Add a `(no-infer ...)` action that prevents inference of targets and
  dependencies in actions. (ocaml/dune#3456, fixes ocaml/dune#2006, @roddyyaga)

- Correctly infer targets for the `diff?` action. (ocaml/dune#3457, fixes ocaml/dune#2990, @greedy)

- Fix `$ dune print-rules` crashing (ocaml/dune#3459, fixes ocaml/dune#3440, @rgrinberg)

- Simplify js_of_ocaml rules using js_of_ocaml.3.6 (ocaml/dune#3375, @hhugo)

- Add a new `ocaml-merlin` subcommand that can be used by Merlin to get
  configuration directly from dune instead of using `.merlin` files. (ocaml/dune#3395,
  @voodoos)

- Remove experimental variants feature and make default implementations part of
  the language (ocaml/dune#3491, fixes ocaml/dune#3483, @rgrinberg)
rgrinberg added a commit to rgrinberg/opam-repository that referenced this pull request Jun 7, 2020
…lugin, dune-private-libs and dune-glob (2.6.0)

CHANGES:

- Fix a bug where valid lib names in `dune init exec --libs=lib1,lib2`
  results in an error. (ocaml/dune#3444, fix ocaml/dune#3443, @bikallem)

- Add and `enabled_ if` field to the `install` stanza. Enforce the same variable
  restrictions for `enabled_if` fields in the `executable` and `install` stanzas
  than in the `library` stanza. When using dune lang < 2.6, the usage of
  forbidden variables in executables stanzas with only trigger a warning to
  maintain compatibility. (ocaml/dune#3408 and ocaml/dune#3496, fixes ocaml/dune#3354, @voodoos)

- Insert a constraint one the version of dune when the user explicitly
  specify the dependency on dune in the `dune-project` file (ocaml/dune#3434 ,
  fixes ocaml/dune#3427, @diml)

- Generate correct META files for sub-libraries (of the form `lib.foo`) that
  contain .js runtime files. (ocaml/dune#3445, @hhugo)

- Add a `(no-infer ...)` action that prevents inference of targets and
  dependencies in actions. (ocaml/dune#3456, fixes ocaml/dune#2006, @roddyyaga)

- Correctly infer targets for the `diff?` action. (ocaml/dune#3457, fixes ocaml/dune#2990, @greedy)

- Fix `$ dune print-rules` crashing (ocaml/dune#3459, fixes ocaml/dune#3440, @rgrinberg)

- Simplify js_of_ocaml rules using js_of_ocaml.3.6 (ocaml/dune#3375, @hhugo)

- Add a new `ocaml-merlin` subcommand that can be used by Merlin to get
  configuration directly from dune instead of using `.merlin` files. (ocaml/dune#3395,
  @voodoos)

- Remove experimental variants feature and make default implementations part of
  the language (ocaml/dune#3491, fixes ocaml/dune#3483, @rgrinberg)
rgrinberg added a commit to rgrinberg/opam-repository that referenced this pull request Jun 7, 2020
…lugin, dune-private-libs and dune-glob (2.6.0)

CHANGES:

- Fix a bug where valid lib names in `dune init exec --libs=lib1,lib2`
  results in an error. (ocaml/dune#3444, fix ocaml/dune#3443, @bikallem)

- Add and `enabled_ if` field to the `install` stanza. Enforce the same variable
  restrictions for `enabled_if` fields in the `executable` and `install` stanzas
  than in the `library` stanza. When using dune lang < 2.6, the usage of
  forbidden variables in executables stanzas with only trigger a warning to
  maintain compatibility. (ocaml/dune#3408 and ocaml/dune#3496, fixes ocaml/dune#3354, @voodoos)

- Insert a constraint one the version of dune when the user explicitly
  specify the dependency on dune in the `dune-project` file (ocaml/dune#3434 ,
  fixes ocaml/dune#3427, @diml)

- Generate correct META files for sub-libraries (of the form `lib.foo`) that
  contain .js runtime files. (ocaml/dune#3445, @hhugo)

- Add a `(no-infer ...)` action that prevents inference of targets and
  dependencies in actions. (ocaml/dune#3456, fixes ocaml/dune#2006, @roddyyaga)

- Correctly infer targets for the `diff?` action. (ocaml/dune#3457, fixes ocaml/dune#2990, @greedy)

- Fix `$ dune print-rules` crashing (ocaml/dune#3459, fixes ocaml/dune#3440, @rgrinberg)

- Simplify js_of_ocaml rules using js_of_ocaml.3.6 (ocaml/dune#3375, @hhugo)

- Add a new `ocaml-merlin` subcommand that can be used by Merlin to get
  configuration directly from dune instead of using `.merlin` files. (ocaml/dune#3395,
  @voodoos)

- Remove experimental variants feature and make default implementations part of
  the language (ocaml/dune#3491, fixes ocaml/dune#3483, @rgrinberg)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants