Skip to content

Conversation

@aantron
Copy link
Contributor

@aantron aantron commented Oct 17, 2024

This spiritually resurrects #1181 by @gpetiot which stalled, as I understand it, due to lack of a Dream release that would make dialect support ergonomic. Because of the Dream release, the present PR doesn't need a wrapper around dream_eml.

It should also make #2764 unnecessary.

@cuihtlauac
Copy link
Collaborator

Thanks @aantron

In #1181, @emillon made this comment:

Sidenote, I had a look at the diff in this PR. If I understand correctly you're replacing N rules with each 1 dependency and 1 target, by a single rule that has N dependencies and N targets.

What's the status now? Are all the .eml going to be processed if a single one changes?

@aantron
Copy link
Contributor Author

aantron commented Oct 18, 2024

AFAIK that comment referred to an approach that was being used in that PR (12c9134#diff-85098e336ea5f1f8dca91cda539ff5f46db068c37b959eb70a32d02bc591941fL6) that had all the template outputs depend on all the template inputs, before @gpetiot switched to using a Dune dialect in the commit from that PR that I am linking in this comment. I also had that concern for a moment before I realized that, but AFAIK there is no reason to suspect that Dune dialects have this downside.

Copy link
Collaborator

@sabine sabine left a comment

Choose a reason for hiding this comment

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

LGTM thanks @aantron!

@sabine sabine merged commit 719c3a3 into ocaml:main Oct 25, 2024
3 checks passed
sabine pushed a commit to ggsmith842/ocaml.org that referenced this pull request Nov 29, 2024
sabine pushed a commit to ggsmith842/ocaml.org that referenced this pull request Dec 10, 2024
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.

3 participants