search

JuliaRegistries / RegistryCI.jl

Continuous integration (CI) tools for Julia package registries, including registry consistency testing, automatic merging (automerge) of pull requests, and automatic TagBot triggers
https://juliaregistries.github.io/RegistryCI.jl/stable
Other
31 stars 30 forks source link

add `read_only` mode #383

Closed ericphanson closed 3 years ago

ericphanson commented 3 years ago

closes https://github.com/JuliaRegistries/RegistryCI.jl/issues/382

I thought this would be way harder! But also I could've missed something.

I wonder if we can test this by adding a github workflow with read-only permissions like we talked about in https://github.com/JuliaRegistries/General/issues/35514, and then run the regular integration tests on that except with read_only set to true. What do you think?

DilumAluthge commented 3 years ago

I wonder if we can test this by adding a github workflow with read-only permissions like we talked about in JuliaRegistries/General#35514, and then run the regular integration tests on that except with read_only set to true. What do you think?

Yeah. I'm about to open a PR demo-ing the read-only token. Once I open that, want to take a look and see if you can figure out how to repurpose it?

DilumAluthge commented 3 years ago

It's going to be tricky for you to figure out how to do integration tests with a read-only token.

Because the mechanism I use in https://github.com/JuliaRegistries/General/pull/35519 to generate a read-only token is a specific mechanism (https://docs.github.com/en/actions/reference/workflow-syntax-for-github-actions#permissions) that only works for the GITHUB_TOKEN supplied automatically by GitHub.

But in the integration tests, I don't actually use the GITHUB_TOKEN. I use a PAT that I generated for my bot user.

ericphanson commented 3 years ago

Ah ok. Maybe we can make a new PAT with read-only perms then?

DilumAluthge commented 3 years ago

I think as a first pass, let's just leave this untested? Since we will be testing it on General. And then we can add tests to this later. (Can you open an issue so we don't forget?)

ericphanson commented 3 years ago

Ah ok, that's fine by me. Let's wait until tomorrow to merge this though; it's late here and it would be good to get @GunnarFarneback's review if possible too.

DilumAluthge commented 3 years ago

While waiting for Gunnar's review, let's make sure CI passes.

bors try

bors[bot] commented 3 years ago

try

Build failed:

ericphanson commented 3 years ago

bors try

bors[bot] commented 3 years ago

try

Build succeeded:

GunnarFarneback commented 3 years ago

I haven't checked if it covers all possible outputs but it certainly makes the right kind of changes and what's there looks right.

ericphanson commented 3 years ago

Alright, let's give it a go! If we run it with a read-only token we should get errors if I missed something here.

bors merge

bors[bot] commented 3 years ago

Build succeeded: