revisiting DENYWARNINGS separate build job (can we combine some?)

[petertseng]

On the policy level, the policy for the example solutions is:

• Must compile without warnings on stable.
• Must compile, but possibly with warnings, on beta.
• Doesn't even need to compile on nightly.

The act of testing all three channels has been here since #85.
The policy on warnings has been here since #107, in response to a time where I introduced warnings in #81 and they were not caught.

On an implementation level, we implement this by having six builds (the product of {stable, beta, nightly} * {normal build, DENYWARNINGS=1 build}) and allowing failures from the appropriate levels.
(We also use a seventh build slot for benchmarks, but they do not figure into this discussion).

---

Let's discuss: Do we desire any changes to the policy?

I don't think so. I think we desire to continue compiling without warnings on stable, because they usually indicate something worth looking at.

I think the most potentially-contentious point would be whether to allow warnings on beta. I think situations like AsciiExt in #411 mean it is wise to allow them, as a transitionary strategy, but I could be wrong on that. If warnings should be disallowed on beta, please at least propose a solution that would have allowed both 1.22 and 1.23 to pass CI (probably just an allow(unused_imports)).

---

Next, let's discuss: Is there a better implementation that implements our desired policy, whatever that may be? For example, perhaps we can combine the normal build and DENYWARNINGS build for stable.

Assuming it doesn't make our test code too hard to understand, I would support an implementation that uses fewer build jobs, since there is a limit to just five jobs running at a time across the entire Exercism organisation (!).

Having DENYWARNINGS separate means we can tell at a glance in Travis CI results whether something failed to compile versus compiled but with warnings, so it seems it's still helpful for both beta and nightly. Thus, we may only be able to save one build slot; maybe not worth whatever extra code it would take.

I'd be glad to hear other ideas.

---

Appendix: Note that our policy for stubs has very recently changed to indicate that they should always compile without warnings: #478

[coriolinus]

Why cut down or merge the builds? Presumably it's to get results back from Travis more quickly. In that case, I'd suggest eliminating the basic nightly builds. They validate Rust's claim of avoiding breaking changes within major versions, but they are by definition unstable; results may vary over time for reasons which have very little to do with the actual code under consideration. I just don't see the inherent value in them. Both BENCHMARK and CLIPPY runs need to use nightly anyway, so we'll still have advance notice if a nightly version breaks compatibility.

I like keeping DENYWARNINGS separate.

[petertseng]

Yes, I think my main motivation is to get results back faster. That means it'd be somewhat interesting to go to 5 by removing the nightly builds. If CLIPPY became its own run, that would bring it up to 6. So in our final desired state, I suppose we still would have more than 5.

I don't think I can provide data to show that this is hurting us, especially since our builds don't take an unreasonably long time.

I don't think I'll be able to justify action, because I wouldn't remove the nightly builds yet until the CLIPPY build is merged. Unless I hear something in the next week or so I'm going to table the discussion (by closing the issue).

[ijanos]

I do not see the point of keeping the nightly builds especially if we want to speed things up.

[petertseng]

I tried to think of a reason why I would say "No! I really want to keep the nightly builds" but I actually didn't think of a good one. The only thing I can think of is that I used them in #411 but only retroactively. It never actively helped me find any problems, only figure out when in nightly a certain thing started breaking, and:

1. A search using my preferred search engine would have been more helpful to do that.
2. That's not really useful information because we don't really make an effort to look at nightly.
3. That's not really useful information because we're not really going to take action if something breaks in nightly because we don't know whether it'll be rolled back or kept (we can take action for beta because we have more restricted expectations).