Allow warnings to be hidden

[jennyrobertsonbbc]

Summary

Fix issue #303. Allow warnings to be hidden in the same way that errors can be hidden.

Details

I made the addWarning method work in the same way as addError. I also added a test.

I added Warnings hidden to the output:

Motivation and Context

Fix issue #303

How Has This Been Tested?

I added a test for the new code which is very similar to the test for hiding errors.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

• I've added tests for my code
• My change requires a change to the documentation.
• I have updated the documentation accordingly.

[andymsuk]

Looks pretty neat! Can we get a feature test added? Can base it on features/hiding_errors.feature?

[andymsuk]

Looks good to me from a code point of view.

@EmmaJP / @micmath / @JamieKnightBBC - any thoughts/comments from you?

[JamieKnightBBC]

Looks good to me. A handy feature which aligns things nicely.

From a code perspective, and this is more a style thing than anything, there’s a line:

if (this.hide) {

I’m not sure what exactly that value is and the condition being checked for. Should it be undefined? True, false, 0 etc.

I’d consider making this more explicit to aid code readability and robustness.

[jennyrobertsonbbc]

Looks good to me. A handy feature which aligns things nicely.

From a code perspective, and this is more a style thing than anything, there’s a line:

if (this.hide) {

I’m not sure what exactly that value is and the condition being checked for. Should it be undefined? True, false, 0 etc.

I’d consider making this more explicit to aid code readability and robustness.

Thanks for your feedback :). I copied the existing function (addError) when I made addWarning and thats how it was in there. I am happy to try to rename the variable in both functions to make it clearer.

Its a reference to how the hide works in the setting, e.g.

page("http://localhost:54321/errors_in_orb_modules.html", {
        hide: "@id='orb-modules'"
      }) 

[andymsuk]

@JamieKnightBBC Yeah, this.hide is used elsewhere. I think it's fine - it's just checking for the presence of the hide option (checking for a truthy value) and a few lines down, it looks at the value to determine what warnings to hide.

[andymsuk]

@JamieKnightBBC - happy to approve?

[ermi-ltd]

Heyo,

Good with me. Was a small style question. Good to have it match what everything else is doing :)

J&L

[ermi-ltd]

Gah, wrong account again. My fault for replying from my phone. blush

[github-actions]

This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request.