game-ci / unity-builder

Build Unity projects for different platforms
https://github.com/marketplace/actions/unity-builder
MIT License
852 stars 250 forks source link

Hotfix: Fix version checking in image-tag #640

Closed AndrewKahr closed 8 months ago

AndrewKahr commented 8 months ago

Changes

Checklist

github-actions[bot] commented 8 months ago

Cat Gif

codecov-commenter commented 8 months ago

Codecov Report

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

Project coverage is 36.97%. Comparing base (0c16aab) to head (9b7bf61).

:exclamation: Current head 9b7bf61 differs from pull request most recent head 84b41f7. Consider uploading reports for the commit 84b41f7 to get more accurate results

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/game-ci/unity-builder/pull/640/graphs/tree.svg?width=650&height=150&src=pr&token=eRCPplc2s1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=game-ci)](https://app.codecov.io/gh/game-ci/unity-builder/pull/640?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=game-ci) ```diff @@ Coverage Diff @@ ## main #640 +/- ## ======================================= Coverage 36.97% 36.97% ======================================= Files 77 77 Lines 3121 3121 Branches 645 645 ======================================= Hits 1154 1154 Misses 1824 1824 Partials 143 143 ``` | [Files](https://app.codecov.io/gh/game-ci/unity-builder/pull/640?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=game-ci) | Coverage Δ | | |---|---|---| | [src/model/image-tag.ts](https://app.codecov.io/gh/game-ci/unity-builder/pull/640?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=game-ci#diff-c3JjL21vZGVsL2ltYWdlLXRhZy50cw==) | `49.45% <100.00%> (ø)` | |
webbertakken commented 8 months ago

Not sure I agree with the reasoning on this one. Just because they add a new version scheme that's actually not so very different doesn't meant we have to make logic more implicit and call it out of scope. In the other PRs, we also still use a regex. The argument "if they change things again" is generally not a great one in software development either.

I'd prefer to keep specific rather than implicit and not architecting for things that might never be.