Closed the-horo closed 4 months ago
This is fantastic work, thank you! Main concern is
gdmd_sha
should have an option for latest. Haven't fully reviewed the tests as it's pretty massive, but overall looks very good, thanks!One note, README says:
Changes from v2
Should it be
Changes from v1
or are we going for v3 so soon?
Depends on how we want to perceive the v2
branch. If we consider it as being a normal release then yes, the action should be bumped to v3
as there has been one breaking change:
compiler: gdmd
is no longer supportedThis could be fixed by reallowing that input and making DC
point to DMD
to preserve the generated environment. Now that I consider, setting DMD
on top of DC
does sound like a breaking change since users may have been using that variable.
Keep in mind that I say this with the previous PR which I've made in mind that ended up breaking other projects' CI because of the changes.
If the v2
branch is perceived as v2-alpha
, so a prerelease, then yeah, it's fine to keep it at v2
. Or at least that how I think semver is supposed to work for actions
Regarding v2, it's not an -alpha
one unfortunately (https://github.com/dlang-community/setup-dlang/releases/tag/v2.0.0) but I don't think it would add much value to the users to bump to v3 so early.
I've tried to address your issues (except for the indentation style) let me know if you're fine with the solutions.
I spotted that I accidentally removed the gdc CI tests when introducing $DMD
but now their back
Resetting the GITHUB_ENV
and GITHUB_PATH
environment variables is a cleaner way to run the unittests during CI compared to importing internal functions.
@the-horo : Will try to review again tonight / tomorrow but I expect this can go in soon. Also invited you to the project.
Thanks. Feel free to work on your pace as I'll have the time to fix stuff if you find anything broken.
Thanks a lot @the-horo ! The nice commit history and the amount of testing is greatly appreciated.
Addresses:
As a summary of changes:
DC
to action will setDMD
to point to the dmd wrapper of the compilercompiler: gdmd
input has been removed, gdmd is now always installed alongside gdces2022
fromes2017
npm test
(on any platform). The current workflow when contributing can be: