Skip to content

feat: better composite action#611

Merged
AtomicFS merged 20 commits intomainfrom
feat/better-composite-action
Mar 18, 2025
Merged

feat: better composite action#611
AtomicFS merged 20 commits intomainfrom
feat/better-composite-action

Conversation

@AtomicFS
Copy link
Copy Markdown
Collaborator

@AtomicFS AtomicFS commented Mar 4, 2025

I am really annoyed by the amount of necessary copy-pasting when using firmware-action in GitHub CI. For some complex firmware stacks, when users want various combinations and use multiple matrix strategies, this complexity explodes quite fast. And most of the workflow then is just cache upload, cache download, artifact upload, artifact download.

This PR is addressing this. However because GitHub CI has a lot of limitations, and many questionable design decisions, it is not straight forward process. Maybe one way would be to implement GitHub API interface, but I do not want o make that nor maintain that. More rant and details in action.yml comments. Also see #84 for more information (== more rant).

Must be merged after #592

To function, this PR relies on features introduced in #603

This PR is rather a big deal

It changes significant parts which did not change for a long time. I don't think it will break anything, but we should test this before deploying.

As such, this change should be just drop-in replacement, it should still work even if users do not change anything. The example tests that we are running seem to work fine after some cleanup of redundant code.

Besides these example tests I am also trying it out in firmware-action-example PR. As you can see there, this PR poses a significant simplification and reduction of code needed in the workflow. For example of functional multi-job workflow see run 13686309056

Fixes #84

@github-actions github-actions bot added documentation Improvements or additions to documentation feature New feature or request testing Testing related dependencies Pull requests that update a dependency file go Pull requests that update Go code github_actions Pull requests that update GitHub Actions code module/coreboot labels Mar 4, 2025
@AtomicFS AtomicFS force-pushed the feat/better-composite-action branch 7 times, most recently from f2c1528 to af922fc Compare March 5, 2025 15:57
@AtomicFS AtomicFS force-pushed the feat/better-composite-action branch 8 times, most recently from 4d69816 to ee145ee Compare March 5, 2025 22:07
@AtomicFS AtomicFS marked this pull request as ready for review March 5, 2025 22:08
@AtomicFS AtomicFS requested a review from MDr164 as a code owner March 5, 2025 22:08
@AtomicFS AtomicFS enabled auto-merge March 5, 2025 22:08
@AtomicFS AtomicFS force-pushed the feat/better-composite-action branch from 699da59 to ca5d93a Compare March 6, 2025 10:40
@AtomicFS AtomicFS force-pushed the feat/better-composite-action branch from ca5d93a to c2bfe5b Compare March 11, 2025 09:30
@AtomicFS AtomicFS requested a review from MDr164 March 11, 2025 13:17
AtomicFS added 12 commits March 17, 2025 15:53
Signed-off-by: AtomicFS <vojtech.vesely@9elements.com>
- just some minor adjustments

Signed-off-by: AtomicFS <vojtech.vesely@9elements.com>
- I am adding another task to the Taskfile in the test directory to
  simulate GitHub CI environment and persuade firmware-action to act
  like it is in CI

Signed-off-by: AtomicFS <vojtech.vesely@9elements.com>
Signed-off-by: AtomicFS <vojtech.vesely@9elements.com>
AI-Generated: true
AI-Model: claude-3.5-sonnet
Signed-off-by: AtomicFS <vojtech.vesely@9elements.com>
AI-Generated: true
AI-Model: claude-3.5-sonnet
Signed-off-by: AtomicFS <vojtech.vesely@9elements.com>
Signed-off-by: AtomicFS <vojtech.vesely@9elements.com>
Signed-off-by: AtomicFS <vojtech.vesely@9elements.com>
Signed-off-by: AtomicFS <vojtech.vesely@9elements.com>
AI-Generated: true
AI-Model: claude-3.5-sonnet
Signed-off-by: AtomicFS <vojtech.vesely@9elements.com>
Signed-off-by: AtomicFS <vojtech.vesely@9elements.com>
Signed-off-by: AtomicFS <vojtech.vesely@9elements.com>
@AtomicFS AtomicFS force-pushed the feat/better-composite-action branch from 16b7599 to 9b33b87 Compare March 17, 2025 14:55
@AtomicFS AtomicFS requested a review from MDr164 March 17, 2025 15:42
Copy link
Copy Markdown

@pointbazaar pointbazaar left a comment

Choose a reason for hiding this comment

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

Should be good to go with a few small changes.

Comments are mostly about not adding options unless they are required,
and naming options with prefixes to make them easier to group and understand
which wrapped action they will go into.

I really like the migration guide which will be very helpful to users.

@AtomicFS AtomicFS requested a review from pointbazaar March 18, 2025 10:23
@AtomicFS AtomicFS force-pushed the feat/better-composite-action branch from cbdabd4 to d7fe0a3 Compare March 18, 2025 10:40
pointbazaar
pointbazaar previously approved these changes Mar 18, 2025
Copy link
Copy Markdown

@pointbazaar pointbazaar left a comment

Choose a reason for hiding this comment

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

Ok, if some of the options are just passed to upload-artifact and download-artifact in a transparent way, then it is fine, since we are not adding any feature there.
Might have gone overboard a little with my preference for simplification in my last review.

I see the prefix has been added to clarify this for the user.
To further clarify, we can remove all documentation of the flags passed to
the upload and download actions, and just link to their respective documentation.

This should be easier for the user, since the information is not duplicated.
I think then it should be good to go 👍🏼

Signed-off-by: AtomicFS <vojtech.vesely@9elements.com>
@AtomicFS
Copy link
Copy Markdown
Collaborator Author

AtomicFS commented Mar 18, 2025

@pointbazaar I actually did forgot one option, just added it for completion.

@AtomicFS AtomicFS requested a review from pointbazaar March 18, 2025 11:32
Copy link
Copy Markdown

@pointbazaar pointbazaar left a comment

Choose a reason for hiding this comment

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

Looks good to me.

  • nice prefix for all the flags related to a wrapped action
  • link to the docs of the wrapped action
  • documentation of which version of that action is being wrapped

👍🏼

@AtomicFS
Copy link
Copy Markdown
Collaborator Author

@MDr164 your approval is still required since you are the owner.

@AtomicFS AtomicFS added this pull request to the merge queue Mar 18, 2025
Merged via the queue into main with commit e94ebb8 Mar 18, 2025
328 of 336 checks passed
@AtomicFS AtomicFS deleted the feat/better-composite-action branch March 18, 2025 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file documentation Improvements or additions to documentation feature New feature or request github_actions Pull requests that update GitHub Actions code go Pull requests that update Go code module/coreboot testing Testing related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

github-action: Allow to specify files in artefacts as input argument

3 participants