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

Hotfix for the setup action #56

Merged
merged 14 commits into from
Sep 12, 2024
Merged

Conversation

mmhat
Copy link
Contributor

@mmhat mmhat commented Sep 7, 2024

In #47 we moved the setup of the get-tested tool to an own reusable action called the setup-get-tested action.
There, the setup-get-tested is referenced in the get-tested action using a relative path ./setup-get-tested as it was assumed that this resolves to the location of the of the cloned get-tested repository when the get-tested is used by another project.
It turns out the assumption was wrong, hence we need to reference the setup-get-tested action in the get-tested action using the OWNER/REPO[/PATH]@REF syntax.

@mmhat
Copy link
Contributor Author

mmhat commented Sep 7, 2024

One problem remains: When someone wants to use a specific version of the get-tested action, say Kleidukos/get-tested@123, the setup-get-tested action used internally should be the one of the same ref, i.e. Kleidukos/get-tested/setup-get-tested@123. Ideally we don't want to hardcode the ref in there, but apparently there is no (clean) way to do that.
See actions/runner#895 for the related issue.

@Kleidukos
Copy link
Owner

Kleidukos commented Sep 9, 2024

Should we roll-back the setup-get-tested thing this gets resolved by GitHub then?

@Kleidukos Kleidukos mentioned this pull request Sep 11, 2024
@mmhat
Copy link
Contributor Author

mmhat commented Sep 11, 2024

Should we roll-back the setup-get-tested thing this gets resolved by GitHub then?

I don't think that is necessary. For now I pinned the used setup-get-tested action to v0.1.8.0 and it can stay on this version as long as we don't change the setup-get-tested action: Note that the downloaded get-tested binary is "opaque" to the setup-get-tested action -- We do not make any assumptions about any functionality present/absent in the binary.
This solution is IMHO a bit awkward, but not completely terrible.

The failing check is completely unrelated to the changes here; There appears to be some problem with the get-tested-head job.

@Kleidukos
Copy link
Owner

@mmhat would you mind rebasing your commits on main?

@mmhat mmhat force-pushed the hotfix-setup-action branch from eeb5a9a to a1be066 Compare September 12, 2024 21:12
@mmhat
Copy link
Contributor Author

mmhat commented Sep 12, 2024

@Kleidukos I rebased the changes. I also used an commit SHA to pin the setup-get-tested action used in the main action.
Before, I used a tag, but the only release that contained the setup-get-tested action was broken and removed accordingly.
We should go back to using a stable release tag in the main action once a new (working) release is published.

@Kleidukos Kleidukos merged commit f4643ba into Kleidukos:main Sep 12, 2024
11 checks passed
@mmhat mmhat deleted the hotfix-setup-action branch September 14, 2024 21:01
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.

2 participants