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.

Sign up for GitHub

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

Jump to bottom

fix: remove use of format in favour of inline bash #2

Merged
merged 5 commits into from
Dec 4, 2023
Merged

fix: remove use of format in favour of inline bash #2

merged 5 commits into from
Dec 4, 2023

Conversation

quotidian-ennui
Copy link
Contributor

@quotidian-ennui quotidian-ennui commented Nov 6, 2023 •
edited
Loading

Motivation

Resolves #1

Changes

  • Use inline bash to derive the supported platform_urls
  • Fixed a minor copy & paste error around platform descs

Test run : https://github.com/quotidian-ennui/install-tool-from-github-release/actions/runs/6772382912

@quotidian-ennui
fix: remove use of format
e8a7713 
Remove use of inline 'format' which doesn't appear to work with
nested templating in favour of a simplistic base step output.
@quotidian-ennui
fix: correct the name of the test
d8465b7
@quotidian-ennui
Copy link
Contributor Author

@stevehipwell -> Steve, you might want to have a CODEOWNERS or similar to assign yourself to review this ;)

stevehipwell
stevehipwell requested changes Nov 29, 2023
View reviewed changes
Copy link
Contributor

@stevehipwell stevehipwell left a comment

Choose a reason for hiding this comment

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

I've added a nit comment for the bash script. Could you also split the PR in half so we can get the action changes merged ASAP.

RE the tests I think a matrix job driven from a YAML file processed in a pre-job would be the best way to make this extensible.

action.yml Outdated Show resolved Hide resolved
@quotidian-ennui quotidian-ennui requested a review from a team as a code owner November 29, 2023 14:18
@quotidian-ennui
Copy link
Contributor Author

quotidian-ennui commented Nov 29, 2023 •
edited
Loading

Ok then, I committed your suggestions, and I removed the test-install-tool yaml; I'll raise a new PR with a matrix style (eventually).

stevehipwell
stevehipwell approved these changes Dec 4, 2023
View reviewed changes
Copy link
Contributor

@stevehipwell stevehipwell left a comment

Choose a reason for hiding this comment

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

Thanks @quotidian-ennui!

@stevehipwell stevehipwell merged commit 3fc3577 into action-stars:main Dec 4, 2023
1 check passed
@quotidian-ennui quotidian-ennui deleted the fix/template branch December 4, 2023 19:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stevehipwell stevehipwell stevehipwell approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

filename_format_linux does not handle templating
2 participants
@quotidian-ennui @stevehipwell