Add ada_toml 0.4.0 #1008
Add ada_toml 0.4.0 #1008
Conversation
|
|
||
| [gpr-set-externals] | ||
| ADA_TOML_BUILD_MODE = "prod" |
There was a problem hiding this comment.
Hi @pmderodat, this should be left unset so clients of the library can define it. You can of course use that as default in your gpr file, so clients are not forced to choose either. I realize now this is inherited from previous releases, so I apologize for previous oversights.
There was a problem hiding this comment.
Hello, thank you for the review! No problem :-)
There was a problem hiding this comment.
Hi, I don't agree here ^^
The default value in ada_toml.gpr is dev.
Most people will just want to use this library and not work on it, so it's better to default to performance and lower binary size.
Also the "dev" mode enables warnings as errors, so there's a good chance this library won't compile with future compilers that will introduce new warnings.
So for me, it's important to either:
- Change the default in
ada_toml.gpr - Set the build mode in the release manifest
- Use the Alire crate config system
There was a problem hiding this comment.
Mmm, so you mean that users should not have the option to even enable dev? Because if set to prod with gpr-set-externals, any attempt to change it will cause a conflict.
I didn't check the default, that makes sense that it be prod.
I don't see why the config system is needed here.
There was a problem hiding this comment.
FTR I would prefer that the default in the GPR file stays dev, because it’s the most convenient in developer mode, which is what I would expect most git clone consumers to do. It would be really nice if Alire was able to have its own default in its spec, and then let Alire users override it if necessary.
There was a problem hiding this comment.
I that case I strongly recommend to bring back the "prod" build mode in the 0.4.0 release manifest
[gpr-set-externals]
ADA_TOML_BUILD_MODE = "prod"
|
Merged, thanks. |
|
Great, thank you very much! |
No description provided.