Conversation
moritzkiefer-da
force-pushed
the
cocreature/generate-dar-resources
branch
from
6e622bc to
98928be
Compare
[ci] Signed-off-by: Moritz Kiefer <moritz.kiefer@purelyfunctional.org>
moritzkiefer-da
force-pushed
the
cocreature/generate-dar-resources
branch
3 times, most recently
from
0c4573d to
6c2419c
Compare
moritzkiefer-da
force-pushed
the
cocreature/generate-dar-resources
branch
from
6c2419c to
ea515cf
Compare
| import com.digitalasset.canton.discard.Implicits.DiscardOps | ||
|
|
||
| object DarResourcesGenerator { | ||
| def render(): String = { |
There was a problem hiding this comment.
There are a bunch of libraries to produce actual scala ASTs instead of doing string concatenation. I honestly have a hard time justifying any of them here. This seems much simpler 🤷
| uses: ./splice-shared-gha/.github/actions/nix/run_bash_command_in_nix | ||
| with: | ||
| cmd: git add . && git diff --staged --exit-code test*.log | ||
| cmd: git add . && git diff --staged --exit-code test*.log '**/DarResources.scala' |
There was a problem hiding this comment.
did confirm that this does indeed fail CI as expected
| uses: ./splice-shared-gha/.github/actions/sbt/execute_sbt_command | ||
| with: | ||
| cmd: "Test/compile lint updateTestConfigForParallelRuns" | ||
| cmd: "Test/compile lint updateTestConfigForParallelRuns updateDarResources" |
There was a problem hiding this comment.
- I did consider instead making this a proper SBT code generator that automatically runs when you compile. I decided against it because it seems like we run clean more often than we change Daml models so not having to rely on SBT caching for this seems better.
- It doesn't seem to be that slow, around 20s maybe (and we could make it faster) so putting it here in the CI job seemed fine.
| import java.nio.file.Path | ||
| import scala.util.Using | ||
|
|
||
| object DarResources { |
There was a problem hiding this comment.
This is a 1:1 copy of the existing DAR resources. I think the next step here would be to auto-generate this as well from the dars we have in the dars directory. That seems to naturally fit into #3667
|
@isegall-da this is not particularly pretty, it does solve the issue though. As usual I'm a bit short on time (was doing this while blocked on LSU and DABFT and I'm getting unblocked there) and I expect #3667 will change some aspects of this. Feel free (even more so than usual) that this needs more work to be mergeable. Will definitely delay it but while it's obviously a nice speed up it's also not an urgent change. |
2 participants
[ci]
Pull Request Checklist
Cluster Testing
/cluster_teston this PR to request it, and ping someone with access to the DA-internal system to approve it./hdm_teston this PR to request it, and ping someone with access to the DA-internal system to approve it.PR Guidelines
Fixes #n, and mention issues worked on using#nMerge Guidelines