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

determine default branch #278

Merged
merged 1 commit into from Jun 16, 2020
Merged

determine default branch #278

merged 1 commit into from Jun 16, 2020

Conversation

@ericsciple
Copy link
Collaborator

@ericsciple ericsciple commented Jun 15, 2020

Fixes #210
Fixes #221

@@ -110,7 +106,7 @@ export function getInputs(): IGitSourceSettings {
core.debug(`recursive submodules = ${result.nestedSubmodules}`)

// Auth token
result.authToken = core.getInput('token')
result.authToken = core.getInput('token', {required: true})
Copy link
Member

@joshmgross joshmgross Jun 15, 2020

Choose a reason for hiding this comment

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

Isn't this a breaking change? Does the action need a token if checking out a public repo?

If we're making this required, we should update the action.yml file with required: true for this input as well.

Copy link
Collaborator Author

@ericsciple ericsciple Jun 15, 2020

Choose a reason for hiding this comment

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

Today it already needs a token but fails with a bad error #221

Copy link
Collaborator Author

@ericsciple ericsciple Jun 15, 2020

Choose a reason for hiding this comment

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

Generally it doesnt need to be specified, because the default value works.

However we've seen cases where it's empty:

  • Folks have set it to ''
  • Due to fork PR a secret doesnt get mapped in

Copy link
Member

@joshmgross joshmgross Jun 15, 2020

Choose a reason for hiding this comment

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

If it's empty, can I still checkout from a public repo?

Copy link
Collaborator Author

@ericsciple ericsciple Jun 15, 2020

Choose a reason for hiding this comment

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

No today it fails like this:

/usr/bin/git -c protocol.version=2 fetch --no-tags --prune --progress --no-recurse-submodules --depth=1 origin +559278e5d4074a5495dceca4d32edaa1acc01724:refs/remotes/origin/master
##[error]fatal: could not read Username for 'https://github.com': terminal prompts disabled

@@ -8,7 +8,7 @@ inputs:
description: >
The branch, tag or SHA to checkout. When checking out the repository that
triggered a workflow, this defaults to the reference or SHA for that
event. Otherwise, defaults to `master`.
event. Otherwise, uses the default branch.
token:
Copy link
Member

@joshmgross joshmgross Jun 15, 2020

Choose a reason for hiding this comment

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

Could we update this to have required: true?

Copy link
Collaborator Author

@ericsciple ericsciple Jun 16, 2020

Choose a reason for hiding this comment

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

It's not required that the user enter a value. It has a default.

@portlek
Copy link

@portlek portlek commented Jun 16, 2020

Fixes #279 and #280 too.

src/git-source-provider.ts Outdated Show resolved Hide resolved
thboop
thboop approved these changes Jun 16, 2020
Copy link
Collaborator

@thboop thboop left a comment

LGTM

@ericsciple ericsciple force-pushed the users/ericsciple/m265branch branch from 4ec7ea5 to 9d2f5b3 Jun 16, 2020
@ericsciple ericsciple merged commit 00a3be8 into master Jun 16, 2020
6 checks passed
@ericsciple ericsciple deleted the users/ericsciple/m265branch branch Jun 16, 2020
@gr2m
Copy link

@gr2m gr2m commented Jun 16, 2020

Thanks for implementing this! 🎉

return yield retryHelper.execute(() => __awaiter(this, void 0, void 0, function* () {
core.info('Retrieving the default branch name');
const octokit = new github.GitHub(authToken);
const response = yield octokit.repos.get({ owner, repo });

Choose a reason for hiding this comment

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

@ericsciple We are using this action on our repository's .wiki repo, and our respective action has been failing recently as the API returns "Not Found" for those. Could this be an oversight here?

Copy link
Collaborator Author

@ericsciple ericsciple Jun 17, 2020

Choose a reason for hiding this comment

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

Thanks for the heads up! And sorry for the disruption :(

I will look into the scenario shortly...

Choose a reason for hiding this comment

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

Thanks! Fortunately was an easy fix on our end :)

Is it even possible to change the wiki default branch name? If not, maybe for those repositories the old hard-coded "logic" could be maintained for now.

kpeterson-sf added a commit to sf-bt-neteng/checkout that referenced this issue Oct 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

6 participants