Stream: genomics/committers
Topic: Updates to IG after FMG feedback
Kevin Power (Mar 21 2022 at 14:30):
Hi all - I reviewed some feedback we got from FMG after our publication request, and made some changes over the weekend and this morning. The changes were pretty simple, but very wide spread. I welcome any/all reviews and feedback.
Kevin Power (Mar 21 2022 at 19:38):
@Bret H added an example for a DiagnosticImplication, and used 'derivedFrom()' as a set of logical references, such as the following:
- derivedFrom[0].display = "Variant 1"
- derivedFrom[0].identifier.system = "http://hospital.example.org"
- derivedFrom[0].identifier.value = "11"
We do limit the 'derivedFrom' to Variant, Genotype, Haplotype. Anyone have concerns with using logical references? Of course, FHIR allows for it. I will plan to 'ignore' the message we are seeing in the QA for now, unless someone objects.
Bret H (Mar 21 2022 at 21:00):
was the QA message a warning that is being seen after a ignore has been removed? I followed other examples both from our IG and Core FHIR. So, I'm curious what the QA signal is. Don't recall seeing it.
Kevin Power (Mar 21 2022 at 21:05):
A QA message was generated by how we do the logical reference. Upon review, if the QA message if a warning or informational, we can decide if it is acceptable. We indicate it is acceptable by adding the error message to the 'ignoreWarnings.txt' file found in the IG, at which point the Publisher will no longer report that QA message.
In this case, to help prove the point of keeping the items we ignore as specific as we can, I had used a really generic 'hey, ignore all slice info QA messages for DiagnosticImplication' - so that is why you didn't see it show up in the QA when you added it and did the build.
Bret H (Mar 21 2022 at 21:06):
so it's an informational QA statement?
Kevin Power (Mar 21 2022 at 21:11):
Correct.
I will add that even for informational messages, it is still best practice to evaluate each QA message. Once we validate the usage is appropriate and the QA message can be ignored, we add the message (as specific as possible) to the 'ignoreWarnings.txt' file along with a comment as to why we think it is OK.
Bret H (Mar 21 2022 at 22:12):
Thank you!
Patrick Werner (Mar 22 2022 at 15:08):
thanks Kevin, looking good!
Patrick Werner (Mar 22 2022 at 15:49):
i removed the redundant 0 usages from warnings: https://github.com/HL7/genomics-reporting/pull/58
Patrick Werner (Mar 22 2022 at 15:50):
cleaned my cache, updated the publisher, works (also mac)
Kevin Power (Mar 22 2022 at 15:50):
Welp, we will see if the cloud build throws the warnings again :smile:
Patrick Werner (Mar 22 2022 at 15:50):
i got some 'The string value contains text that looks like embedded HTML tags. If this content is rendered to HTML without appropriate post-processing, it may be a security risk' in my local build
Patrick Werner (Mar 22 2022 at 15:51):
If the errors are comming back, ill createn an issue and fix the warnings again
Kevin Power (Mar 22 2022 at 15:57):
Looks like the build worked fine, no other warnings. So maybe it was a caching thing.
Kevin Power (Mar 22 2022 at 17:03):
Welp, FYI. When I download the change from @Patrick Werner and build locally, I still get the warnings (with or without my cache). However, I left just the two that Patrick left and for now will ignore the warnings I see. I pushed up some improved comments, and we will see if the cloud build still works correctly.
Kevin Power (Mar 22 2022 at 17:05):
And the cloud build just finished, and no warnings were logged (meaning things worked as expected). So perhaps the weirdness is something just for me to deal with :smile:
Patrick Werner (Mar 23 2022 at 09:45):
that's really odd. We can also add it back if you prefer
Kevin Power (Mar 24 2022 at 14:13):
Nah that is OK. It looks cleaner this way. If anyone else ever notices it, we can add it back or investigate further.
Last updated: Apr 12 2022 at 19:14 UTC
