pydicom / dicom-validator

Simple DICOM validator based on DocBook DICOM specs
MIT License
25 stars 11 forks source link

Release windows executable as part of release-deploy #110

Closed smjoshiatglobus closed 4 months ago

smjoshiatglobus commented 4 months ago
codecov-commenter commented 4 months ago

:warning: Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 89.93%. Comparing base (e8644e2) to head (bf12ac0). Report is 26 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #110 +/- ## ========================================== + Coverage 89.82% 89.93% +0.10% ========================================== Files 15 15 Lines 1632 1669 +37 ========================================== + Hits 1466 1501 +35 - Misses 166 168 +2 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

smjoshiatglobus commented 4 months ago

Yes, please! Keeping my fingers crossed!

mrbean-bremen commented 4 months ago

Nope, didn't work. I will check later...

smjoshiatglobus commented 4 months ago

Bummer! I do see that the executable was built correctly in the job. I double-checked and confirmed that action-gh-release specifies using / for directory separator even for Windows. I still have much to learn!

mrbean-bremen commented 4 months ago

I have hacked it to work (by amending the commit and re-creating the release several times). The actual problem was an error in the GH release action that I also missed: you had put an hyphen before the file names:

      with:
        files: |
          - dist/validate_iods.exe
          - dist/dump_dcm_info.exe

which should have been:

      with:
        files: |
          dist/validate_iods.exe
          dist/dump_dcm_info.exe

(e.g. these are not sub-items, but just the file names) I actually replaced it with:

      with:
        files: dist/*.exe

I also divided the workflow into two separate jobs, so that the pypi release depends on the successful executable creation, to avoid the problem for future releases. I did this by force-pushing the changes to avoid to have to make a new PyPi release, not really the clean way... but we do have a release including the executables now, so thank you for the contribution!

smjoshiatglobus commented 4 months ago

Ah! My bad! Thank you for fixing it. I can now point others to the release page for downloading the executables!