Skip to content

Conversation

@ericsciple
Copy link
Contributor

@ericsciple ericsciple commented Jun 15, 2020

Fixes #210
Fixes #221


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

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
Contributor 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
Contributor 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
Contributor

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
Contributor Author

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

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
Contributor

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
Contributor Author

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 commented Jun 16, 2020

Fixes #279 and #280 too.

Copy link
Contributor

@thboop thboop left a comment

Choose a reason for hiding this comment

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

LGTM

@ericsciple ericsciple force-pushed the users/ericsciple/m265branch branch from 4ec7ea5 to 9d2f5b3 Compare June 16, 2020 17:17
@ericsciple ericsciple merged commit 00a3be8 into master Jun 16, 2020
@ericsciple ericsciple deleted the users/ericsciple/m265branch branch June 16, 2020 17:41
@gr2m
Copy link

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
Contributor Author

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.

This was referenced Mar 15, 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.

could not read Username for 'https://github.com': terminal prompts disabled support renamed default branches

7 participants