-
Notifications
You must be signed in to change notification settings - Fork 13
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
Various improvements #1
base: master
Are you sure you want to change the base?
Conversation
@@ -63,13 +64,18 @@ | |||
|
|||
github_user = github_repo.split('/')[0] | |||
github_slug = github_repo.split('/')[1] | |||
|
|||
git_working_folder = github_slug + "-" + gh_branch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I was having issues with it trying to commit into the same path as the existing checkout was in (which was a different branch).
'https://{}@github.com/{}.git'.format(github_token, github_repo), | ||
github_slug, | ||
'https://x-access-token:{}@github.com/{}.git'.format(github_token, github_repo), | ||
git_working_folder, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This means you can just use a bog standard ${{ secrets.GITHUB_TOKEN }}
and you don't have to have a special personal access token or anything.
I can give this a new input name and check for either if you'd rather for backwards compatibility?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I forget why, however I'm pretty sure I have tried using GitHub token and failed (some permission error i forgot). I will try it again when I have the time, maybe GitHub have fix the problem
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know if it's because you need x-access-token
, but it works now:
peternewman/Date-Holidays-GB@14f5883
https://github.com/peternewman/Date-Holidays-GB/runs/2242370226?check_suite_focus=true#step:14:174
Is there any reason why this PR is still a draft? The changes look good, and I look forward to see them merged. |
I think at the time I hadn't used it, but there's a WIP PR making use of it here (the apt bit is fine, just some other tidying up to do there): I'm not sure if the x-access-token bit would be a breaking change, or if some filenames could be valid on their own, but not when globbed e.g. foo[abc]bar.deb, but they're probably fairly low risk. |
Wondering if this action could be extended to be triggered in |
@barnumbirr you probably want to raise this as an issue, rather than a comment on my PR. However you can already do at least part of that, if you look at https://github.com/mjemmeson/Date-Holidays-GB/blob/master/.github/workflows/test.yml I'm building https://github.com/jonasbn/perl-date-holidays-super within my repo. In that case I'm fetching it via CPAN (a perl tool), but a git clone would work too. So you could certainly build on a regular basis within B and publish from there. Whether you can build based on a commit in A and publish to B probably depends on the token scope, it may well be possible. |
No description provided.