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.

Sign up for GitHub

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

Jump to bottom

Setup code style fix action #9057

Closed
wants to merge 33 commits into from
Closed

Setup code style fix action #9057

wants to merge 33 commits into from

Conversation

fisker
Copy link
Sponsor Member

@fisker fisker commented Aug 25, 2020

#9051 (comment)

  • I’ve added tests to confirm my change works.
  • (If changing the API or CLI) I’ve documented the changes I’ve made (in the docs/ directory)
  • (If the change is user-facing) I’ve added my changes to changelog_unreleased/*/pr-XXXX.md file following changelog_unreleased/TEMPLATE.md.
  • I’ve read the contributing guidelines.

✨Try the playground for this PR✨

@thorn0
Copy link
Member

thorn0 commented Aug 27, 2020

This needs some kind of protection against Prettier's idempotence bugs, otherwise it might end up in an infinite loop.

@fisker
Copy link
Sponsor Member Author

fisker commented Aug 27, 2020

It seems a dead end, can't commit to the fork.

@fisker
Copy link
Sponsor Member Author

fisker commented Aug 27, 2020

Have you seen this https://github.com/stefanzweifel/git-auto-commit-action#using-the-action-in-forks-from-public-repositories ?

@thorn0
Copy link
Member

thorn0 commented Aug 27, 2020

Yep, but as you can see we came very close: remote: Permission to fisker/prettier.git denied to github-actions[bot].

You can only make commits on pull request branches that:

  • are opened in a repository that you have push access to and that were created from a fork of that repository
  • are on a user-owned fork
  • have permission granted from the pull request creator
  • don't have branch restrictions that will prevent you from committing

(source)

At this point, we can probably give push access to github-actions[bot] or one of us can generate a personal access token and add it to the repo as a secret, but both of these options look extremely insecure to me.

@fisker
Copy link
Sponsor Member Author

fisker commented Aug 27, 2020

I agree.

@fisker fisker closed this Aug 27, 2020
@fisker fisker reopened this Aug 27, 2020
@thorn0
Copy link
Member

thorn0 commented Aug 27, 2020

In this PR, you added a new workflow file and it got immediately executed. Is it because you have push access? Or does it work the same way for anyone's PRs? In the latter case, anybody can steal all the secrets by simply opening a PR.

@fisker
Copy link
Sponsor Member Author

fisker commented Aug 27, 2020

It's not working, that commit caused by opening a new PR on my fork 😄

fisker#711

@fisker
Copy link
Sponsor Member Author

fisker commented Aug 27, 2020 •
edited

In this PR, you added a new workflow file and it got immediately executed.

I don't know.

Normally, GitHub seems require a same name workflow exists, to run?

@thorn0
Copy link
Member

thorn0 commented Aug 27, 2020

It didn't work because with pull_request_target, the workflow files are taken from the base branch of the PR (i.e. master in our case). We don't have this workflow in master yet, that's why it doesn't work.

fisker and others added 2 commits August 27, 2020 19:32
@fisker @thorn0
Setup code style fix action
e638a94 
Test

commit_message

with ref

Update code-style.yml

Experimenting

see actions/checkout#61 (comment)

Experimenting 2

Experimenting 3

Try `pull_request_target`

Fix code style

Revert "Fix code style"

This reverts commit 2ecc65b.

Remove other tasks
@thorn0
Revert "Remove other tasks"
639c719 
This reverts commit 3357c1a.

Revert "Revert "Fix code style""

This reverts commit 18053a3.

Revert "Fix code style"

This reverts commit 2ecc65b.

Revert "Try `pull_request_target`"

This reverts commit 556328c.

Revert "Experimenting 3"

This reverts commit 1d77b5d.

Revert "Experimenting 2"

This reverts commit 33186fc.

Revert "Experimenting"

This reverts commit c57f8c0.

Revert "Update code-style.yml"

This reverts commit 92797dc.

Revert "with ref"

This reverts commit 48863af.

Revert "commit_message"

This reverts commit 5bde13f.

Revert "Setup code style fix action"

This reverts commit 6f7bb03.
@thorn0 thorn0 changed the base branch from master to tmp-9057 August 27, 2020 16:34
@thorn0 thorn0 marked this pull request as draft August 27, 2020 16:38
@thorn0
Copy link
Member

thorn0 commented Aug 27, 2020 •
edited

Seems to work! Let me open one more PR (from my fork this time) against the tmp-9057 branch to test it one more time.

---> #9083

It works again. How cool is this?

@fisker
Copy link
Sponsor Member Author

fisker commented Aug 27, 2020

Cool!

@fisker fisker changed the base branch from master to tmp-9057 August 28, 2020 01:09
@fisker fisker changed the base branch from tmp-9057 to master August 28, 2020 01:58
@lipis
Copy link
Member

lipis commented Sep 17, 2020

Interesting one..! Are we waiting anything for this one? Worst case scenario we can always remove it later.

@fisker
Copy link
Sponsor Member Author

fisker commented Sep 17, 2020

#9083

Base automatically changed from master to main January 23, 2021 17:13
@fisker fisker mentioned this pull request Apr 22, 2022
Open
4 tasks
@fisker
Copy link
Sponsor Member Author

fisker commented Dec 4, 2023

Close in favor of #15749

@fisker fisker closed this Dec 4, 2023
@fisker fisker deleted the code-style-format branch December 4, 2023 15:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexander-akait alexander-akait alexander-akait left review comments

@lipis lipis lipis approved these changes

Assignees
No one assigned
Labels
do-not-merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@fisker @thorn0 @lipis @alexander-akait