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

Clean up GitHub Actions. #2921

Merged
merged 8 commits into from
Dec 7, 2023
Merged

Clean up GitHub Actions. #2921

merged 8 commits into from
Dec 7, 2023

Conversation

mitchell-as
Copy link
Contributor

@mitchell-as mitchell-as commented Dec 5, 2023

TaskDX-2339 Audit use of `state run preprocess` in github actions

This task turned into a general GitHub Actions cleanup. It really needed to be done.

Update verify.yml
@mitchell-as mitchell-as closed this Dec 5, 2023
@mitchell-as mitchell-as reopened this Dec 5, 2023
@github-actions github-actions bot changed the base branch from master to version/0-43-0-RC1 December 5, 2023 16:43
language: bash
if: ne .Shell "cmd"
value: |
if ! type "gozip" &> /dev/null; then
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was originally needed for the offline-installer, but that was moved to its own repo, so this is no longer needed.

@mitchell-as mitchell-as requested a review from MDrakos December 5, 2023 18:53
@mitchell-as mitchell-as marked this pull request as ready for review December 5, 2023 18:53
Copy link
Member

@MDrakos MDrakos left a comment

Choose a reason for hiding this comment

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

Overall this looks fine to me. Just one very mine inclusion I think.

env:
ACTIVESTATE_CI: true
Copy link
Member

Choose a reason for hiding this comment

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

It makes sense to remove this one here but I think we should include it on workflows that use the state tool as consumers of our analytics could be using this now or may want to in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree. I don't see a usage of state tool without the ACTIVESTATE_CI environment variable defined. Would you please point it out?

Copy link
Member

Choose a reason for hiding this comment

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

Just searched and it looks like it's all there. Sorry, when I saw it removed I didn't look in the other files.

@mitchell-as mitchell-as requested a review from MDrakos December 7, 2023 18:31
@mitchell-as mitchell-as merged commit 601f7f5 into version/0-43-0-RC1 Dec 7, 2023
7 checks passed
@mitchell-as mitchell-as deleted the mitchell/dx-2339 branch December 7, 2023 19:38
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