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

ci: Normalise slash-containing github.head_ref values #2166

Merged
merged 1 commit into from
Nov 17, 2024

Conversation

shymega
Copy link
Contributor

@shymega shymega commented Nov 14, 2024

This fixes CI on #2121.

Copy link

github-actions bot commented Nov 14, 2024

Build size and comparison to main:

Section Size Difference
text 374576B -16B
data 948B 0B
bss 63504B 0B

@NeroBurner NeroBurner added bug Something isn't working maintenance Background work labels Nov 14, 2024
@shymega shymega force-pushed the ci-fix-normalise-head-ref branch 2 times, most recently from 47c8d6c to f0122ac Compare November 14, 2024 23:56
@shymega
Copy link
Contributor Author

shymega commented Nov 14, 2024

@mark9064 If you could run the workflow one more time - I was testing in zsh locally, not bash - tested in bash with the recent force-push, and it works locally. Should be fine now.

@mark9064
Copy link
Member

No worries, CI development is the worst because testing in prod is basically the only way :P

@shymega shymega force-pushed the ci-fix-normalise-head-ref branch from f0122ac to 8ac0c9b Compare November 15, 2024 00:54
@shymega
Copy link
Contributor Author

shymega commented Nov 15, 2024

One more time - then if that fails, I'll take a look either before or after working hours today.

@NeroBurner NeroBurner added this to the 1.15.0 milestone Nov 15, 2024
@FintasticMan
Copy link
Member

I think you might need to escape the forward slash in the substitution. I.e. ${ref_name//\//-}

@shymega
Copy link
Contributor Author

shymega commented Nov 15, 2024 via email

@shymega shymega force-pushed the ci-fix-normalise-head-ref branch from 8ac0c9b to e7e4868 Compare November 15, 2024 19:48
@shymega
Copy link
Contributor Author

shymega commented Nov 15, 2024

Ah, meant to push! Done now.

@shymega shymega force-pushed the ci-fix-normalise-head-ref branch from e7e4868 to 73be6e5 Compare November 15, 2024 21:38
Copy link
Member

@FintasticMan FintasticMan left a comment

Choose a reason for hiding this comment

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

You've replaced github.head_ref with github.ref_name, meaning that the output is different.

@shymega shymega force-pushed the ci-fix-normalise-head-ref branch from 73be6e5 to f6ab516 Compare November 15, 2024 23:02
@shymega shymega requested a review from FintasticMan November 15, 2024 23:06
Copy link
Member

@FintasticMan FintasticMan 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 now!

@shymega
Copy link
Contributor Author

shymega commented Nov 15, 2024

Excellent! Once we get this landed, I'll rebase #2121 - and then we can merge that, too!

@JF002 JF002 merged commit 4dd0d60 into InfiniTimeOrg:main Nov 17, 2024
5 checks passed
@JF002
Copy link
Collaborator

JF002 commented Nov 17, 2024

Thanks for your contribution @shymega

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working maintenance Background work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants