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

Add options for the pants-init action #21

Merged
merged 8 commits into from
May 5, 2023
Merged

Add options for the pants-init action #21

merged 8 commits into from
May 5, 2023

Conversation

bryanwweber
Copy link
Contributor

This adds options for:

  • Configuring the cache location on the runner
  • Configuring the GH_HOST for enterprise instances

It also does a little cleanup on some of the other steps. @jsirois encouraged me to submit this after this conversation on Slack

Copy link
Member

@kaos kaos left a comment

Choose a reason for hiding this comment

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

Thanks. lgtm overall bar the typos ;)

init-pants/action.yaml Outdated Show resolved Hide resolved
init-pants/action.yaml Outdated Show resolved Hide resolved
Thanks for catching the typos!

Co-authored-by: Andreas Stenius <git@astekk.se>
@bryanwweber
Copy link
Contributor Author

Thanks for catching the typos @kaos! I just commited your suggestions, so should be all good now.

Copy link
Member

@kaos kaos left a comment

Choose a reason for hiding this comment

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

lgtm.

I'd prefer a second pair of eyes though, as I'm not that familiar with GH actions.

Copy link
Sponsor Contributor

@benjyw benjyw left a comment

Choose a reason for hiding this comment

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

I assume you've manually tested this, and are sure that the GH_TOKEN change is good?

Copy link
Member

@cognifloyd cognifloyd left a comment

Choose a reason for hiding this comment

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

This is great. Making the cache paths configurable is superb, and cleaning up the if: statements and the use of runner.temp is excellent.

I'm concerned about injecting GH_TOKEN into GITHUB_ENV however, and I've left a comment about that.

Also I had a couple other minor questions.

init-pants/action.yaml Outdated Show resolved Hide resolved
init-pants/action.yaml Outdated Show resolved Hide resolved
init-pants/action.yaml Show resolved Hide resolved
@bryanwweber
Copy link
Contributor Author

@cognifloyd Thanks for the review! I confirmed the changes worked with thegh-host input set, so I think it's good to go. Thanks!

Copy link
Member

@cognifloyd cognifloyd left a comment

Choose a reason for hiding this comment

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

That looks better. Thanks for testing my suggestion -- I'm glad it worked!

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.

5 participants