Skip to content

Test - automatic CS fix + commit (in a PR from forked repo) #1098

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

Closed
wants to merge 11 commits into from

Conversation

OskarStark
Copy link
Member

Lets see if a PR receives an automatic CS fix

@OskarStark
Copy link
Member Author

OskarStark commented Sep 23, 2020

@localheinz any idea why this works with a branch of the same repo in #1097, but not a remote branch? 🤔

This check should be green and a commit should be done

@jordisala1991
Copy link
Member

You should add pull_request_target to the action trigger

@OskarStark
Copy link
Member Author

I tried, as you can see in the latest checks down below... 🤔

CleanShot 2020-09-24 at 07 18 03

Maybe @stefanzweifel can help us 😄

@stefanzweifel
Copy link

👋

The php-cs-fixer runs fail, as the "Test/ PHP 8.0" run fails first: https://github.com/sonata-project/dev-kit/runs/1154581328#step:10:4
The php-cs-fixer runs never actually make it to the "fix and commit" part of the workflow.

@OskarStark
Copy link
Member Author

Hey, thanks for reaching out 👋

The php-cs-fixer runs fail, as the "Test/ PHP 8.0" run fails first:

In this PR #1097 PHP 8-0 was failing too 🤔

@OskarStark
Copy link
Member Author

AFAIK @localheinz is working on ergebnis/playground#230, too, but in this case he is fixing a local branch instead of a pull request branch from the outside repository 🤔

@OskarStark
Copy link
Member Author

OskarStark commented Sep 30, 2020

@stefanzweifel, now the PHP 8.0 build is green, but the others are still failing :(

@stefanzweifel
Copy link

It seems GitHub can't clone this repo anymore: https://github.com/sonata-project/dev-kit/pull/1098/checks?check_run_id=1186863006#step:3:38

There's an open issue on the actions/checkouts-repo which seems related to this; but without fix.
Maybe by setting some more env variables you can see more about the error: actions/checkout#143 (comment)

@OskarStark
Copy link
Member Author

@stefanzweifel I added:

        env:
          GIT_TRACE: 1
          GIT_CURL_VERBOSE: 1

@stefanzweifel
Copy link

@OskarStark Thanks! I still have no good evidence why the checkout in the linter runs fails.

I think it could be related that the run is triggered from a forked repository. 🤔
For both the pull_request and pull_request_target-triggers, GitHub tries to checkout the sonata-project/dev-kit with the ref OskarStark-patch-1

In git-auto-commit we had a similar problem recently (stefanzweifel/git-auto-commit-action#117 (comment)).
My guess is, that GitHub changed something on their side around how forks work with GitHub Actions - or didn't update (or improved) the docs yet.

Maybe you could give it a go by removing the with/ref-part in the workflow?
The other steps in your workflow can checkout the code just fine. 🤔

@OskarStark
Copy link
Member Author

Thanks for the feedback 🙏

Maybe you could give it a go by removing the with/ref-part in the workflow?

Sure, I did, lets see...

@OskarStark
Copy link
Member Author

Nothing... 😞

@stefanzweifel
Copy link

Interesting.
pull_request fails as it tries to push the branch from your forked repo to the base repo. Fails as the branch OskarStark-patch-1 doesn't exist on https://github.com/sonata-project/dev-kit.
https://github.com/sonata-project/dev-kit/pull/1098/checks?check_run_id=1288423771#step:5:14

pull_request_target fails, as it tries to checkout the branch from the fork in the base repo. Also fails as the branch OskarStark-patch-1 doesn't exist on https://github.com/sonata-project/dev-kit.
https://github.com/sonata-project/dev-kit/pull/1098/checks?check_run_id=1288423570#step:3:4

I've just tagged a new version of git-auto-commit with adds a checkout_options-option. You could theoretically use it with 'checkout_options: -b' to force the action to create a new branch and push that branch to the remote. But this will probably also not work here as it's a fork.

The README of git-auto-commit has recently been updated to point these issues with forks out. Actions like this which run a linter/fixer should probably run on a push-trigger on the main-branch of the repo. Makes life thousand times easier. 😅
https://github.com/stefanzweifel/git-auto-commit-action#using-the-action-in-forks-from-public-repositories

(But I understand that it would be great if it would work in a PR)

@OskarStark
Copy link
Member Author

You could theoretically use it with 'checkout_options: -b' to force the action to create a new branch and push that branch to the remote.

I try, thanks 👍

Actions like this which run a linter/fixer should probably run on a push-trigger on the main-branch of the repo. Makes life thousand times easier.

Well I agree here, but it makes diffs harder to read, creates comments by people who doesn't know this + if you revert something ... its not so clean IMHO.

@OskarStark OskarStark changed the title Test Test - automatic CS fix + commit (in a PR from forked repo) Oct 29, 2020
@franmomu franmomu mentioned this pull request Jan 3, 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.

4 participants