Skip to content

removed redundant code from masterfhirvalidation.yml, including python and poetry - #21

Merged
ryma2fhir merged 13 commits into
mainfrom
update/yml
Oct 12, 2023
Merged

removed redundant code from masterfhirvalidation.yml, including python and poetry#21
ryma2fhir merged 13 commits into
mainfrom
update/yml

Conversation

@ryma2fhir

Copy link
Copy Markdown
Contributor

This has been tested on a fork in my repo and is still working correctly.

…asterfhirvalidation.yml. This is the same repo but has been renamed at some point.
Comment thread .github/workflows/masterfhirvalidation.yml
Comment thread .github/workflows/masterfhirvalidation.yml Outdated
@declankieran

Copy link
Copy Markdown
Contributor

I'd be surprised if this step

      - name: Cache node modules
        uses: actions/cache@v3
        with:
          path: ~/.npm
          key: ${{ runner.os }}-build-cache-npm-packages-${{ hashFiles('**/package-lock.json') }}

is providing any noticeable speed-up in the execution of the workflow. I'd suggest trying without and removing it if there is no significant difference.

…mmands to keep in actions for ease of reading, makefile also deleted
@ryma2fhir

Copy link
Copy Markdown
Contributor Author

removed cache, jkd/node/npm install as not needed. makefile deleted as not needed. Saved approx 1 min per action, but the biggest times are the build and run fhir validator steps.

@declankieran declankieran left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually one of the jobs failed and it seems like its the Makefile being removed made it fail

image

In testingbranch.yml

image

Is this just another copy of the validation script?

@ryma2fhir

ryma2fhir commented Oct 11, 2023

Copy link
Copy Markdown
Contributor Author

Actually one of the jobs failed and it seems like its the Makefile being removed made it fail

image

In testingbranch.yml

image

Is this just another copy of the validation script?

Very similar. The last few lines of code are installing, building and testing the IOPS-FHIR-Validation-Service repo and not this one. I have no idea what that is actually testing, but that's for another day. I've replaced the make line for this repo to get it running again. Working as expected (error on FHIR asset not passing test).

@declankieran

Copy link
Copy Markdown
Contributor

I think its now possible just catching a change in the display of some of the codes, not related to the actual running of the script, e.g. http://snomed.info/sct#322236009 has a preferred synonym of "Paracetamol 500 mg oral tablet", not "Paracetamol 500mg tablets" on the current version showing on the snomed ct browser, haven't checked which version you are referencing. Probably worth fixing those just to get a clean run of the validation script to be sure there are no other issues.

@ryma2fhir
ryma2fhir dismissed declankieran’s stale review October 12, 2023 10:59

reverted code back as make was pointing to another repo

@ryma2fhir
ryma2fhir merged commit d1f71bd into main Oct 12, 2023
@ryma2fhir
ryma2fhir deleted the update/yml branch October 12, 2023 12:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants