I think a better analogy is that it’s like wearing safety goggles and a hard hat while you’re building a house and then putting on a suit when you’re showing the completed house to buyers. The warnings are useful to you while you’re building software but not so much to the people trying to use your software.
1 Like
When is unsafe code a problem? While you write it or when you run/release it?
1 Like
Pointing out a problem is only useful when you can solve it, and you can only do that during development.
Having a warning prevent an installation is an issue I had a few time as a user and its hugely frustrating.
4 Likes
I’m arguing for treating errors as warnings at the end of development when checking in code, but not at the beginning or in the middle of development.
Such a setup can certainly be achieved with CI, and would be nicer than what we have.
I think the biggest reason why its not case is dune not being able to replay warning, so you only see them once, except when rebuilding the part of the code where the warning occurs.
This is a defect of dune for sure. I am not sure how difficult it is to fix it, but it would certainly be a big improvement.
1 Like
Sounds like you want a workflow action that rejects pull requests if .ml or .mli files contain [@@@warning “-A”].
1 Like
Sure, but that requires users to be aware of this and in general most people want to configure their tools as little as possible before moving on. So yeah, you can set a git pre-commit hook to check for warnings, but this will be used by maybe 1% of all users. When designing an UX you often need to make sure that the default-case leads to good outcomes instead of telling the user “well, if only you set up your project in this way then it would be better”. Sometimes the less good solution leads to improvements in practice, while still not preventing another setup (like you can disable warnings and set up CI to enable warnings and reject merges if warnings appear but notice that this requires quite a lot of setup and if you develop a project as one person you might not even have a process where each change goes through a PR).
For what its worth, a side-effect of Dune enabling a bunch of warning by errors has actually lead to a lot of these warnings being addressed in the codebases that adopted Dune. I think that’s overall a win for the quality of published software.
3 Likes
Citation needed :–) Perhaps they also have simply been selectively disabled (I would even suspect that the current system entices people to do so).
I think the system can be made subtler than what your post seems to convey. For example all the unused warnings except the unused-var could, for a start, be disabled during development. That way they do not get in the way when you fool around with the code base.
However they could be enabled as errors when you do the build that checks your release tarball actually builds before publication during the release process (assuming dune-release distrib kept that from the topkg distrib worfklow).
Now you will tell me it’s annoying to get warned-error when you are about to do your release and just want to go party but the thing is these warnings are exceptionally easy to handle: just delete or comment out the code. It could even be automated or semi-automated git add -p style (and as you make more and more release you will get accustomed to that happening and factor it in).
This is anecdotal evidence but whenever I dune build Dune projects, they build just fine with warnings as errors. I checked my local projects and couldn’t find any that would override the default setting. Most PRs that converted projects to use Dune that I remember fixed the code that lead to warnings rather than disabling the warnings.
This requires people to use dune-release or topkg however, aka additional setup that people need to do. I don’t know if opam-publish does these checks nor whether people that use other tools do that. In theory opam-repo-ci could attempt to test that, but it would seem a bit odd if opam-repo maintainers were to enforce which warnings should be enabled and which ones should not.
1 Like
Not necessarily. Everything seems always so rigid in the dune world :–)
A first start would be to have a proper lint build profile. You can then let people integrate or use this lint profile in their publication workflow the way they wish. Bonus points if you manage to make the lint profile extensible with user defined criterions/tooling.
And let’s be honest it’s not dramatic if you happen to make a release which includes dead code. I think dune is making a bad usability tradeoff by having all these unused warnings on as errors by default during dev (at least those whose treatment do not require any refactoring).
P.S. There are also other ideas like making a lint profile every nth builds but you need to be careful with these kind of ideas and you’d need an exceptional good UI here (ideas: good explanation of what happened, ignore, ignore until next commit, redo the lint build, etc.). They may end up being annoying if the users cannot make a good mental model of it and appears to them as random.
This brings us back to the original point that having these additional profiles or checks introduces new things for people to do in their deploy/publish pipelines. And most people are just not going to do these things. It doesn’t scale. What does scale is being forced to fix warnings immediately, whether it’s by fixing the actual issue or just disabling the warning.
1 Like
Whose experience is usually… automated.
I’m not sure what exactly scales here for the warnings I’m talking about. Bad usability at scale perhaps.
Anyways no point in trying to suggest things for dune, dune is perfect it seems.
It’s automated after setting up the automation. Which is a manual step. That’s what doesn’t scale. Dune being strict out of the box doesn’t really impede anyone; they can loosen the warnings and errors with a small effort. But they chose the right default and encouraged everyone to think about code quality from day one.
Dune is not perfect, but it’s a massive improvement over what came before it. I’d like to see it continue to improve, especially the documentation.
My conclusion then is that dune doesn’t scale. I think this discussion is a bit ridiculous. Let’s leave it there :–)
What does scale is to always use dune with the release profile because anything else is very frustrating during a quick iteration loops.
I do not want to start all my files with a disable warning flag (that I will need to manually clean-up when I’m done).
I do not want to create all my variable with a _ so I can avoid the unused warning and then rename everything once it’s used.
2 Likes
Modify your toplevel dune file, not your ocaml files.
Something like
(env (dev (flags :standard -warn-error -a)) (release (flags :standard -warn-error a)))
Commit that and it will be the way you want (which is the way I don’t want, so I’d rather dune devs not change the default)
3 Likes
And that is supposed to scale 
. Look if I only count the repos I’m using to publish software on opam I already have 40 repos to modify, only to get a reasonably usable system.
Also note that what I’m suggesting is actually not about disabling warn-error it’s about being smart about the warnings you enable by default and at what point of the software development cycle.
But for that you’d need people who actually understand software usability in terms of workflows, not in term of features.
+1 for the topic
Concerning the warnings as errors discussion:
I’ve been disabling a continuously bigger set of
warnings in all my dune files when creating new dune projects:
- 27 [unused-var-strict]
- 33 [unused-open]
- 32 [unused-value-declaration]
- 26 [unused-var]
- 69 [unused-field]
- 35 [unused-for-index]
The only reason being that I want to start iterating fast, and clean code is not a priority at the start - and warnings as errors are a hinderance for certain kinds of debugging too.
What I think I would actually like instead, is to re-enable these warnings as errors per module - not per dune library - and only do this at the point when the specific module has become relatively stable - not when releasing it.
Also, a point underlining this; some of the warnings can be used for something akin to added typesafety during development. I’ve used the following pattern in specific modules when I wanted to be sure that all record fields of a type was used by all the functions operating on it:
[@@@warning "+27+9"]
[@@@warnerror "+27+9"]
...
let foo ~v =
let V.{
field01 = _;
field02;
field03;
field04;
} = v in
<expr>
...
.. so I would need to make a manual choice of not using field01 inside foo, and it will be an error if some field is not mentioned in expr.
1 Like
Suggestion this one liner as a “solution” when I said I already do dune build -–profile=release means we don’t really have the same opinion about tools usability. Honestly you could have told me to go back to calling ocamlc manually 
This forum is full of people who go out of their way to disable dune default behavior, so at least I’ll take comfort in knowing I’m not alone.
Personally I’d be happy with a more relaxed profile I could set-up as the default globally in my dune config profile.
I must say that I have a very hard time with dune’s decision to change the defaults on warnings as errors. I’m fine with having an opinionated build system, but the point is to minimize the opinionated surface as much as possible and to stay in your lane. There’s absolutely no reason to change the defaults of the compiler with regard to warnings. It can be presented as an extra feature for those who want it, but certainly should not be made the default. This is getting dune designers involved in matters that should be orthogonal to their concerns. One cannot claim to want to make a build system that the whole community will embrace, and then start changing default options that have nothing to do with the build system in a way that will exclude people. I deactivate all dev warnings-as-errors right away when building my projects, and the fact that this requires an incantation rather than being an easy to use switch adds further insult to injury.
Unfortunately, this isn’t the only place where libraries targeted at “everyone” have intruded with decisions in areas which should not concern them, making it harder than it should be for them to be embraced. It seems to be a common malady in our ecosystem.
2 Likes