Add -Werror to CXXFLAGS for dev CI job

[Enchufa2]

As a first step towards #1497, this simply converts warnings into errors for the checks with R devel, which I thought it was the most appropriate one. All checks still pass because I didn't activate any extra warnings yet.

Checklist

• Code compiles correctly
• R CMD check still passes all tests
• Preferably, new tests were added which fail without the change
• Document the changes by file in ChangeLog

[eddelbuettel]

A good start

[Enchufa2]

The only problem with this setup is that, when tests fail due to warnings turned into compilation errors, these issues are not shown in the runner logs, but in R's own install or check logs. So we could add an additional step after the checks that, when these fail, either

• cats the relevant output; or
• uploads the Rcpp.Rcheck folder as an artifact for further inspection.

I personally like the second option.

[eddelbuettel]

Had we discussed this before you altered an existing file 😆 I might have convinced you to start with a new one 😆. If we start from the plain r-ci template r-ci.yaml you note that it already has a part to dump logs in case of failure():

https://github.com/eddelbuettel/r-ci/blob/4316c2c3cd96eb457ab2ca4e04abe52146c9e61c/docs/r-ci.yaml#L36-L38

Adding an artifact is good, and easy to add in a one-off file.

But as always there are more than one way to skin a cat and you are in the driver's seat, so other ideas welcome too.

[Enchufa2]

I thought it would save some CPU cycles and therefore some water if we reused the ci-dev run already in place, but happy to add a new yaml if you think is cleaner. :)

Same applies to cat vs artifact. I personally find the artifact useful, but I'm fine with dumping the logs if you prefer.

[eddelbuettel]

I am open to either approach. What I want to avoid, mostly, is for this to inflict ❌ on routine PRs and updates. If we can avoid that, good. If we cannot, I would think an opt-in is cleaner. Let's see how it goes.

[Enchufa2]

Ok, let's go for the cleaner approach then. :) Give me a sec.

[Enchufa2]

All good then with this approach? Should I merge?

[eddelbuettel]

Yes I think this is a good base layer. I would have called it, say werror.yaml not warning.yaml but that can be changed subsequently too.

So next we start PRs adding additional triggering flags and tried to wind down the number of warnings / errors we get that way to zero?

[Enchufa2]

Yes I think this is a good base layer. I would have called it, say werror.yamlnotwarning.yaml but that can be changed subsequently too.

Said and done.

So next we start PRs adding additional triggering flags and tried to wind down the number of warnings / errors we get that way to zero?

Yes, that's the idea.