Adds some infra to warn on files which changed in the PR but aren't accounted for #2901
Conversation
.github/workflows/CI.yml
Outdated
|
|
||
| # Upload site artifact for forks so it can be deployed by a maintainer on-demand | ||
| - uses: actions/upload-artifact@v3 | ||
| if: github.event.pull_request.base.repo.id != github.event.pull_request.head.repo.id |
There was a problem hiding this comment.
Er, is this not the code which powers the deploy-preview label? https://github.com/microsoft/TypeScript-Website/blob/b03935a55750acfadc496e5fdf4258730c96cb5f/.github/workflows/deploy-preview.yml
Probably, just the deploy is what needs to be deleted?
There was a problem hiding this comment.
Oh yeah, I thought it was doing a more native version of it, will drop next time I get back to the computer
|
This got cleaned up 👍🏻 |
| run: | | ||
| # Exposing this token is safe because the user of it has no other public repositories | ||
| # and has no permission to modify this repository. See DefinitelyTyped #62638 for the discussion. | ||
| TOKEN='ghp_i5wtj1l2AbpFv3OU96w6R' |
There was a problem hiding this comment.
Even after reading through the comment thread at DefinitelyTyped/DefinitelyTyped#62638, I'm still not a fan of using an unprotected token. Making this token public means anyone could use it to attempt to DDoS GitHub and have it falsely attributed to the DangerBotOSS account, or DoS Danger by artificially using up its rate limit. If making this a secret is not viable for usability reasons, are there any other mechanisms that could be employed to avoid exposing the token?
There was a problem hiding this comment.
I'm not really sure why this needs a token at all, either; isn't this just a CI check that can fail and print out the files that were forgotten? a la https://github.com/microsoft/TypeScript/blob/main/.github/workflows/ci.yml#L239
I guess because this tries to print a fancy comment?
There was a problem hiding this comment.
It's because you need a token to make a comment on an issue, yes!
Yep, it's a comment because no-one would read a non-failing CI build for things like warnings
I did explore having a central github app danger/danger-js#1126 but it requires giving too much github access to the bot IMO, and I didn't want to centralize that many people's tokens on my spare time
Co-authored-by: Ron Buckton <[email protected]>
|
Done via another PR. |
Re: #2900
I'm not entirely sure about the fully cleaned bootstrap issues (it was only really meant to be ran once to download sets of files, and not really be needed to run again except for new ts versions) - so I've moved the one thing which definitely was only triggering in bootstrap only from the most recent PR into the build flow, its a tiny codegen and seems reasonable. I think most of the changes from the recent PR came from OSS folks not running the build but just making direct changes.
This PR switches Danger to run on all PRs and removes my old infra for doing static web app deploys ( Azure/static-web-apps#1 (comment) ) using the same code as we use in DT. It won't fail the build, but it will list what files have changed locally at the end of the run and that could be a good indicator of what's going on.