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

Introduce external repository token #357

Merged
merged 3 commits into from
Jan 14, 2021

Conversation

robertbrignull
Copy link
Contributor

@robertbrignull robertbrignull commented Jan 4, 2021

Potential alternative to #273 to demonstrate an idea.

Introduces GitHubApiExternalRepoDetails and GitHubApiCombinedDetails (names very open to changing) to get us type safety on the different types of api token.

Merge / deployment checklist

  • Confirm this change is backwards compatible with existing workflows.
  • Confirm the readme has been updated if necessary.

Closes #273.

Copy link
Contributor

@chrisgavin chrisgavin left a comment

Choose a reason for hiding this comment

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

This does seem like a fairly neat option to maintain the encapsulation. I'm happy with this method instead of #273.

@robertbrignull robertbrignull force-pushed the robertbrignull/external-token-option branch from 44bb170 to 97a70e6 Compare January 12, 2021 12:09
@robertbrignull
Copy link
Contributor Author

I forgot to include a couple of bits of #273 like the changes to the readme and to init/action.yml. I think I've got it all now and this should work but I'm going to give it a manual test before merging. @chrisgavin, could you have another quick look and see if you spot anything else I've missed?

@robertbrignull
Copy link
Contributor Author

I've tested this out locally with a private repository that could only be downloaded by passing the external repository token. It all seems good.

@robertbrignull robertbrignull merged commit f86e200 into main Jan 14, 2021
@robertbrignull robertbrignull deleted the robertbrignull/external-token-option branch January 14, 2021 17:41
@github-actions github-actions bot mentioned this pull request Jan 18, 2021
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