Closed Geod24 closed 9 months ago
✅ PR OK, no changes in deprecations or warnings
Total deprecations: 14
Total warnings: 0
Build statistics:
statistics (-before, +after)
-executable size=5335176 bin/dub
+executable size=5335248 bin/dub
rough build time=66s
@Geod24 Can you explain that in more detail? I don't see how BuildPlatform
has anything to do with "data". If anything, you might want to separate the PackageRecipe
logic from the data part. What is the reasoning for getting rid of the compiler
import?
@s-ludwig : Sure. I want to separate everything that has to do with the PackageRecipe
type definition in its own module.
That means moving dependency
, buildsettings
, packagerecipe
to dub.data
, and making sure those are (almost) leaf modules.
dub.data
is not the greatest package name, I will give you that. But having the separation between data types and logic is what I am after and this will yield much cleaner code because module dependencies will be lower, and code will be easier to understand.
I hope this makes more sense ?
In that case, I would suggest to look into moving getPlatformSettings
out of packagerecipe
and letting BuildPlatform
where it is. It could certainly be argued that getPlatformSettings
is on a level closer to Project
anyway.
Looking into it more, I'm not so convinced anymore that this PR was right. Can I have until year-end to complete what I am working on, and if I cannot make a compelling argument, we revert this ? I don't think it's going to take that long but I want a bit of margin.
@Geod24 Just a reminder so that this doesn't get forgotten.
Good point: https://github.com/dlang/dub/pull/2788
We want to separate the data part, which is exposed to the PackageRecipe, from the actual platform check code, which is used when building.
Note that it's exposed to
PackageRecipe
indirectly, through itscompiler
import, which I aim to remove.