hl7au / au-fhir-test-data

HL7 AU FHIR Test Data
2 stars 6 forks source link

Add initial files #28

Closed jgsuess closed 1 month ago

jgsuess commented 1 month ago

Hello @heathfrankel,

First, thank you for making time to review the PR. It is a bit dated, and maybe that is a good reason for dropping it. I have cloned the source code in this branch, so that should not be an issue.

First the intention: My goal was to create an environment that could execute repeatable test runs of the tool in a CI chain. I do not have access to Windows hosts, as they are expensive in licensing. Hence, I needed to run the tool on Linux. I expected that to be easy, as .Net should cross-compile. However, when trying the tool it broke due to what turned out embedded windows path separators. However, this can be fixed by replacing the backslash with a forward slash, which works on both Windows and Unix.

This led me to recompile. The outcome is the file without the extension you have found. In Unix, executables do not have file name extensions. I placed the file in the branch in the way you did with yours, to stay in line with established convention.

Because the build was not automated, I added build automation for Unix and Windows to github actions and archived the results using artifacts.

Now let me have a look at the issues you mentioned:

  1. There are a set of json files added to the Postman folder, what are these and why are there so many? This folder is intended for Postman collections and environment files for vendor download and use. Adding all these files make it hard to identify these amongst all these new files. --- At the time I started this work, the most recent set of files was only available as a collection embedded in postman. The branch contains a script to obtain these contents to ensure that failures were not due to files being out of date.
  2. Under the Sparked.CSV2FhirMapping folder there is a new backup folder, what is this. --- This folder should probably be removed. I believe that some the mappings also contained the backslash and needed to be changed.
  3. There is also a Csv2Fhir file with no extension, what is this? --- The Unix executable.
  4. There is a UpgradeLog.htm file, why is this added? --- This is the result of setting up the necessary builder. It can be discarded.
  5. Why are the existing source files changed? The updates appear to do with windows path separators being changed to L~l~inux. Will this now affect a windows build not operating? --- yes, no, based on smoke testing. There are no automated tests in this projects that I could use.
  6. Why has the specification.zip been updated? --- I am not aware that it has been.
  7. Similar questions related to changes in Sparked.TestDataClient folder. --- Similar answer re: issues with separator,
  8. Why have resources in generated folder changed? - Relative to which commit? What do you see in the diff?

Please let me know if you want to abandon the MR, as I have said, I only did this to have a basis for my comparison. However, I believe that the automated Github actions build and unix capability may still be of value. In this case, please amend the branch.

jgsuess commented 1 month ago

Oh, and w.r.t. there being no ticket, there is a ticket reference in the right margin: image

In Jira this is towards https://jira.csiro.au/browse/ST-285

heathfrankel commented 1 month ago

"What do you see in the diff" - I see all the changed files when I select the "Files Changed" tab.

Sounds like I'll have to find some time to go through all these changes to understand how they will affect the master, but I will also need to understand if any of this is still required as I am still not clear what the intended outcome was.

Currently I am focusing on preparing for the connectathon. Perhaps we can discuss on the friday after the connectathon.

jgsuess commented 1 month ago

I'm closing the pr but keeping the branch that's easiest I think.