Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for eden_version input in test workflow #954

Merged

Conversation

europaul
Copy link
Contributor

@europaul europaul commented Feb 7, 2024

When the workflow is triggered from the outside, action/checkout defaults to checking out the latest master which is not always desired. For better compatibility with older EVE release the user should be able to specify the version of eden to be used in the test workflow. If no version is specified, the workflow should resert to the default behavior.

@europaul
Copy link
Contributor Author

europaul commented Feb 7, 2024

@uncleDecart this is a WIP, I want to run the tests and see if this implementation works. For the cause of this change see the discussion with @eriknordmark here

@uncleDecart
Copy link
Collaborator

@europaul Ideally, when referencing workflow in EVE here it should reference and use eden files specified in that branch. There was a bug in checkout I thought I fixed it here

@europaul
Copy link
Contributor Author

europaul commented Feb 7, 2024

@europaul Ideally, when referencing workflow in EVE here it should reference and use eden files specified in that branch. There was a bug in checkout I thought I fixed it here

@uncleDecart you are right! But I assume we still haven't found a solution how to backport the fix to 0.9.2 - this is why the jobs are failing. How about creating a branch from 0.9.2, perhaps calling it something similar, backporting the commit with the fix to that branch and then referencing the branch in EVE's 11.0-stable

@uncleDecart
Copy link
Collaborator

make sense

@eriknordmark
Copy link
Contributor

eriknordmark commented Feb 7, 2024

One yetus failure:
::warning file=.github/workflows/test.yml,line=4,col=1::[truthy] truthy value should be one of [false, true]:::warning: file=.github/workflows/test.yml,line=10,col=21::[comments]: too few spaces before comment

When the workflow is triggered from the outside, action/checkout
defaults to checking out the latest master which is not always desired.
For better compatibility with older EVE release the user should be able
to specify the version of eden to be used in the test workflow. If no
version is specified, the workflow should resert to the default behavior.

Signed-off-by: Paul Gaiduk <paulg@zededa.com>
@europaul europaul force-pushed the add-eden-version-input-for-test-workflow branch from 335ef53 to 2d3a0fc Compare February 7, 2024 15:53
@europaul
Copy link
Contributor Author

europaul commented Feb 7, 2024

@uncleDecart it looks like the solution from here doesn't work. See latest comments on the issue and our own test results

Should we use the one proposed in the current PR instead?

Copy link
Contributor

@eriknordmark eriknordmark left a comment

Choose a reason for hiding this comment

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

LGTM

@uncleDecart
Copy link
Collaborator

Looks good to me, but right now we can't see if it's working or not. I'll create small example to check it out and let you know

@uncleDecart
Copy link
Collaborator

@europaul works. I'd suggest we bump checkout to v4, since there's no reason to use 3.5.3 anymore and let's merge this and backport to 0.9.2-stable branch so that we can point EVE 11-stable to 0.9.2-stable Eden (in case we need any other backporting)

@europaul
Copy link
Contributor Author

europaul commented Feb 8, 2024

@europaul works. I'd suggest we bump checkout to v4, since there's no reason to use 3.5.3 anymore and let's merge this and backport to 0.9.2-stable branch so that we can point EVE 11-stable to 0.9.2-stable Eden (in case we need any other backporting)

I think we can leave the checkout version as it is - we still use v3 for all the other workflows for example and I don't see any new features in the latest release that we would need.

@uncleDecart
Copy link
Collaborator

uncleDecart commented Feb 8, 2024

@europaul updating to v4 makes sense, because they update to node20, since node16 EOL is 11 sept. 23

Edit: see here

Since node16 that is used by action/checkout v3 has reached EOL, we upgrade the version to the latest 4.1.1.

Signed-off-by: Paul Gaiduk <paulg@zededa.com>
@europaul
Copy link
Contributor Author

europaul commented Feb 8, 2024

@europaul updating to v4 makes sense, because they update to node20, since node16 EOL is 11 sept. 23

Edit: see here

@uncleDecart you are right! The EOL slipped my mind. Updated to 4.1.1 for whole Eden.

Copy link
Collaborator

@uncleDecart uncleDecart left a comment

Choose a reason for hiding this comment

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

LGTM, will wait for tests and then merge

@uncleDecart uncleDecart merged commit d698974 into lf-edge:master Feb 9, 2024
14 of 16 checks passed
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.

None yet

3 participants