Attach actions from cinaps stanza to @cinaps and @runtest #2831
Conversation
NathanReb
force-pushed
the
cinaps-alias
branch
from
3741eb8
to
8fb1fba
Compare
| (Path.extend_basename fn ~suffix:".cinaps-corrected")) ))) | ||
| (Path.extend_basename fn ~suffix:".cinaps-corrected")) )) | ||
| in | ||
| Super_context.add_alias_action sctx ~dir ~loc:(Some loc) ~stamp:"cinaps" |
There was a problem hiding this comment.
I would suggest to move Alias.cinaps to this module. The reason is cinaps is just an extension and I don't think it's worth hardcoding its existence in places where it's not completely necessary.
There was a problem hiding this comment.
Sure, I'll move it!
There was a problem hiding this comment.
Should it be considered a "standard" alias then?
It seems that currently it can't be made a standard alias from outside the Alias module.
There was a problem hiding this comment.
standard is something that allows the alias not to error when it's empty? is that necessary? I don't think the manually defined cinaps alias has that either. Let's see if @diml has a preference.
There was a problem hiding this comment.
Yeah we discussed that with @emillon but I was wondering whether it should apply here since the cinaps alias can be defined without user explicitly creating it. It feels like it might require to be defined as a standard alias in that regard.
I don't have a strong opinion, either are fine by me!
There was a problem hiding this comment.
I moved the alias to Cinaps and therefore made it a regular alias. I'm happy to move it back to Alias or to expose make_standard if we want to make it that way!
There was a problem hiding this comment.
Making cinaps standard would mean that user can call dune build @cinaps in a project-independent manner, i.e. without having to care whether cinaps is used or not. This is valuable for tests for instance, but a bit less for cinaps given that it is not very widespread.
When you think about it, it would make sense to include cinaps validation in the linting phase. We could do so by attaching it to the lint alias, which is already standard.
Finally, we could also have a general syntax for allowing a recursive alias to be empty. Such as dune build @?cinaps.
In conclusion, for now I wouldn't make cinaps standard. We can always revisit this decision in the future.
NathanReb
force-pushed
the
cinaps-alias
branch
3 times, most recently
from
3b5cc71
to
eef6521
Compare
|
Is the test failure expected? I got it locally and thought something was maybe misconfigured on my side but it looks like an omitted promotion. |
|
My bad. Let me push a fix.
…On Nov 4, 2019, 6:22 PM +0900, Nathan Rebours ***@***.***>, wrote:
Is the test failure expected? I got it locally and thought something was maybe misconfigured on my side but it looks like an omitted promotion.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
NathanReb
force-pushed
the
cinaps-alias
branch
from
eef6521
to
2fda82a
Compare
NathanReb
force-pushed
the
cinaps-alias
branch
from
2fda82a
to
2dd8a5f
Compare
NathanReb
force-pushed
the
cinaps-alias
branch
2 times, most recently
from
24685e9
to
5f57ed9
Compare
|
I also removed the |
NathanReb
force-pushed
the
cinaps-alias
branch
from
5f57ed9
to
20d620b
Compare
|
Just rebased, I'm guessing I can merge once CI is green. |
NathanReb
force-pushed
the
cinaps-alias
branch
from
20d620b
to
5ba8d7a
Compare
|
Sounds good! |
Signed-off-by: Nathan Rebours <[email protected]>
NathanReb
force-pushed
the
cinaps-alias
branch
from
5ba8d7a
to
7df09e7
Compare
…lugin, dune-private-libs and dune-glob (2.1.0) CHANGES: - Attach cinaps stanza actions to both `@runtest` and `@cinaps` aliases (ocaml/dune#2831, @NathanReb) - Add variables `%{lib-private...}` and `%{libexec-private...}` for finding build paths of files in public and private libraries within the same project. (ocaml/dune#2901, @snowleopard) - Add `--mandir` option to `$ dune install`. This option allows to override the installation directory for man pages. (ocaml/dune#2915, fixes ocaml/dune#2670, @rgrinberg) - Fix `dune --version`. The bootstrap didn't compute the version correctly. (ocaml/dune#2929, fixes ocaml/dune#2911, @diml) - Do not open the log file in `dune clean`. (ocaml/dune#2965, fixes ocaml/dune#2964 and ocaml/dune#2921, @diml) - Support passing two arguments to `=`, `<>`, ... operators in package dependencies so that we can have things such as `(<> :os win32)` (ocaml/dune#2965, @diml)
…lugin, dune-private-libs and dune-glob (2.1.0) CHANGES: - Attach cinaps stanza actions to both `@runtest` and `@cinaps` aliases (ocaml/dune#2831, @NathanReb) - Add variables `%{lib-private...}` and `%{libexec-private...}` for finding build paths of files in public and private libraries within the same project. (ocaml/dune#2901, @snowleopard) - Add `--mandir` option to `$ dune install`. This option allows to override the installation directory for man pages. (ocaml/dune#2915, fixes ocaml/dune#2670, @rgrinberg) - Fix `dune --version`. The bootstrap didn't compute the version correctly. (ocaml/dune#2929, fixes ocaml/dune#2911, @diml) - Do not open the log file in `dune clean`. (ocaml/dune#2965, fixes ocaml/dune#2964 and ocaml/dune#2921, @diml) - Support passing two arguments to `=`, `<>`, ... operators in package dependencies so that we can have things such as `(<> :os win32)` (ocaml/dune#2965, @diml)
This attach the cinaps actions to the
@cinapsalias and add a dependency from@runtestto@cinaps! Fixes #2801