Closed ecchilds closed 2 years ago
@weisJ @nightm4re94 The greeting seems to be causing a build failure on this PR.
~@weisJ~ @nightm4re94 The greeting seems to be causing a build failure on this PR.
Oh lord, GitHub actions have an annoying design flaw... The token for PRs from forks is read-only, making the greetings completely useless for the standard use case of commenting on PRs. The web is full of aggravated people and it doesn't seem like we're seeing this change anytime soon.
~@weisJ~ @nightm4re94 The greeting seems to be causing a build failure on this PR.
Oh lord, GitHub actions have an annoying design flaw... The token for PRs from forks is read-only, making the greetings completely useless for the standard use case of commenting on PRs. The web is full of aggravated people and it doesn't seem like we're seeing this change anytime soon.
Yeah, I think it's read only from forks because then someone without permissions could make a malicious github action to spam the PR with hundrends of replies, and you wouldn't be able to close it because the close button is at the bottom.
There's probably other more compelling reasons too... There's probably some sort of privilege escalation issue with that too.
Sure, with the way the first-interaction action works right now, using a readonly token is the only way to prevent that. But not being able to automatically comment on PRs from forks is... not an optimal solution.
For the greetings use case pull_request_target
trigger is precisely what you need. It will run on the current master branch instead, so you don’t have access to the pull request branch, but the greetings should run just fine.
Yeah, I'd recommend giving https://nathandavison.com/blog/github-actions-and-the-threat-of-malicious-pull-requests a quick read. Basically, just use pull_request_target
and do not use actions/checkout
in the workflow.
These functions in GuiComponent currently move child components twice. This happens because they both call setLocation, which moves child components, and then they also move child components themselves. This commit fixes this.