Skip to content

Commit

Permalink
Switch to windows-latest
Browse files Browse the repository at this point in the history
  • Loading branch information
@tzanio
tzanio committed Aug 20, 2022
1 parent c205ed5 commit 0cc02d8
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion build-mfem/action.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ runs:
echo "--- Configure mfem ---"
cd ${{ inputs.mfem-dir }};
if [[ ${{ inputs.build-system }} == 'cmake' ]]; then
if [[ ${{ inputs.os }} == "windows-2022" ]]; then
if [[ ${{ inputs.os }} == "windows-latest" ]]; then

This comment has been minimized.

Sign in to view

Copy link
@pazner

pazner Aug 22, 2022

Member

@tzanio, I think this might break compatibility since branches that are currently using this action will still provide windows-2022 as the inputs.os (since mfem/mfem#3162 hasn't been merged yet).

Maybe better to check ${{ runner.os }} == "Windows" instead? cc: @psocratis

This comment has been minimized.

Sign in to view

Copy link
@tzanio

tzanio Aug 23, 2022

Author Member

I was hoping we can merge this quickly and then merge master in those branches 😄

I do like ${{ runner.os }} == "Windows" much more though, I'll change to that

This comment has been minimized.

Sign in to view

Copy link
@v-dobrev

v-dobrev Aug 23, 2022

Member

As a general rule: do not move tags like this (I assume that's what you did here).

Instead, create a new tag, v2.3 or v2.2.1 or something like that.

This comment has been minimized.

Sign in to view

Copy link
@v-dobrev

v-dobrev Aug 23, 2022

Member

Basically, this action is public and used by other repositories (like GLVis) and changing the fixed version will likely break the CI in those repos.

This comment has been minimized.

Sign in to view

Copy link
@v-dobrev

v-dobrev Aug 23, 2022

Member

Actually, v2.2 is a branch here, not a tag, however, my remarks still apply.

This comment has been minimized.

Sign in to view

Copy link
@pazner

pazner Aug 23, 2022

Member

Yeah, each version of the action is a branch. I think it's OK to push non-breaking fixes (e.g. a URL changed, and the action needs the updated one to work properly), but if possible backwards compatibility should be preserved, and if not then a new versioned branch should be created.
toolchain_file="${VCPKG_INSTALLATION_ROOT}\\scripts\\buildsystems\\vcpkg.cmake"
vcpkg_triplet="x64-windows-static"
fi
Expand Down

0 comments on commit 0cc02d8

Please sign in to comment.