lf-edge / eden

Eden is where EVE and Adam get tried and tested:
https://projecteve.dev
Apache License 2.0
49 stars 47 forks source link

Add support for eden_version input in test workflow #954

Closed europaul closed 7 months ago

europaul commented 7 months ago

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 commented 7 months ago

@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 commented 7 months ago

@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 commented 7 months ago

@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 commented 7 months ago

make sense

eriknordmark commented 7 months ago

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

europaul commented 7 months ago

@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?

uncleDecart commented 7 months ago

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 commented 7 months ago

@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 commented 7 months ago

@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 commented 7 months ago

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

Edit: see here

europaul commented 7 months ago

@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.