Remove "info" status from test report #727
Conversation
…issue712-remove-info
Codecov Report
@@ Coverage Diff @@
## master #727 +/- ##
==========================================
- Coverage 82.61% 82.12% -0.49%
==========================================
Files 29 29
Lines 4181 4174 -7
==========================================
- Hits 3454 3428 -26
- Misses 727 746 +19
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
There's still something wrong with our configuration setup, since the same change (to the config files) is being made in ~3 places. I forget what our current plan/strategy is, but I'd like to make sure we have a plan in place to clean this up rather than just kick the can down the road. @pisuke -- do you know how we were tracking this? @pbatta Maybe the plan was that I was supposed to do something :-) -- but if so, I'd at least like to make sure I have a bug assigned for it! |
|
@pisuke @grafnu a further note on config, the three standard configurations (CI, qualification, remediation) are not completely in sync with each-other currently, with minor differences which shouldn't be there. It's on my list of items to review and resolve with @pisuke, and I'll create a Github issue for it. It's happened because the three files aren't always updated when they should have been, and I did not pick up on it when reviewing some of the test PR's |
|
Ok, great. To be clear, I'd really like to address the underlying
structural problem (they should inherit from a common config), rather than
just make them all in sync! Let's come up with a plan first and then we can
execute... I think the straightforward thing would be an "includes"
mechanism, and then use that!
…On Mon, Jan 4, 2021, 3:21 PM Noureddine ***@***.***> wrote:
@pisuke <https://github.com/pisuke> @grafnu <https://github.com/grafnu> a
further note on config, the three standard configurations (CI,
qualification, remediation) are not completely in sync with each-other
currently, with minor differences which shouldn't be there. It's on my list
of items to review and resolve with @pisuke <https://github.com/pisuke>,
and I'll create a Github issue for it. It's happened because the three
files aren't always updated when they should have been, and I did not pick
up on it when reviewing some of the test PR's
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#727 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAIEPD4QA2YIB7YQP6XFLRTSYJEOTANCNFSM4VSZRG2A>
.
|
|
Ok, I've updated DAQ HEAD with the new include mechanism, so not sure the best path forward. It's "optional" in that it should work just fine without it (this PR more or less as-is), or update this PR to use the new mechanism. Either way, we should do it "soon" so that we can have a solid round of QA on it. |
|
I'm working on one more PR that uses the other file names (e.g.
port_config.json)... Should be done today. I'm just doing the changes to
the code and tests, not all of the docs and folders...
…On Wed, Jan 13, 2021, 3:36 AM Noureddine ***@***.***> wrote:
@grafnu <https://github.com/grafnu> I'll close this one. Will do a new
"config overhaul" PR which includes this PR done properly with the new
include mechanism, along with the config renaming and documentation as
described by @pisuke <https://github.com/pisuke>. This will come soon
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#727 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAIEPDYYZKWGYXNBQMVNAQ3SZWASDANCNFSM4VSZRG2A>
.
|
Issue #712 to remove "info" status and test category from the report.
Instead of an "info" result, these tests will pass if it has been possible to get the information, otherwise fail or skip as appropriate.