Feature/report stale if one stale

[Timple]

Going from:

Report stale as errors unless all stale

to:

If one STALE and no ERROR, report STALE

As a bonus, the message that caused the degradation of the diagnostics is added to the toplevel state. This is useful for reporting mechanisms such as error logs.

[Timple]

Rebased as build failed on #507

[Timple]

Any objections here?

We think it makes more sense to report STALE as summary. Because now you can have an ERROR without any actual ERROR states.

[Timple]

If there are any reasons not to do this, we'd also like to hear. Or should this pass an MPC or something?

[Timple]

Almost a year...

Can someone point us at someone who can shine a light if this is desired or undesired?

[peci1]

I haven't gone through the logic of the implementation, but from a high-level point, I agree with this direction (I'm not a maintainer here, though).

[Timple]

Valid comments. Thank you for the review.

I managed to dispute none and to address them all 🙂

[mfaferek93]

LGTM!

[ct2034]

Thanks for the review @mfaferek93 ! I agree with it and as far as I can follow it, the logic makes sense to me.
Is there a chance we could get an opinion from @asymingt ? (who developed this in #315)

[ct2034]

please fix this one comment

[ct2034]

I made some more tests. ;-)
Could you please merge https://github.com/ros/diagnostics/tree/feature/report-stale-if-one-stale into your branch.
There is one test that is failing for me locally, and I would be interested in your opinion.

[ct2034]

I made some more tests. ;-) Could you please merge https://github.com/ros/diagnostics/tree/feature/report-stale-if-one-stale into your branch. There is one test that is failing for me locally, and I would be interested in your opinion.

Nvm, I think figured the test out. The case was wrong. But please still include them here @Timple.

[ct2034]

Merged in #593

[Timple]

I see I was too late. Thanks for merging.

Not sure about the backports though. It's a breaking change so not sure who would be affected in what way.

[ct2034]

Nvm, it worked out.
I am always in favour of backports to keep the codebase consistent. And especially this case I would primarily treat as a bug, because the old behaviour seemed to not make sense to anyone.