RegistryCI.jl icon indicating copy to clipboard operation
RegistryCI.jl copied to clipboard

run automerge on malformed package names, but exit early

Open ericphanson opened this issue 1 year ago • 1 comments

Motivated by https://github.com/JuliaRegistries/General/pull/99899

Currently, new_package_title_regex regex causes us to decide it's not a new package (nor a new version) and to exit.

Instead, I think AutoMerge should run and post a clear comment that the name is not OK.

Here I've added a new guideline to verify the package name is a valid Julia identifier. We would fail the "normal capitalization" guideline later on, but IMO if we don't meet this basic check we should just exit early rather than proceeding through the list.

ericphanson avatar Jan 30 '24 23:01 ericphanson

Instead, I think AutoMerge should run and post a clear comment that the name is not OK.

I agree, I think this is the right approach.

DilumAluthge avatar Jan 30 '24 23:01 DilumAluthge

I think this is pretty simple and uncontroversial so I'll merge soon

ericphanson avatar Jun 11 '24 23:06 ericphanson

I haven't reviewed the code, but your description sounds like this is the right approach.

DilumAluthge avatar Jun 11 '24 23:06 DilumAluthge

~~Testing is slightly tricky here since this is the first integration test case which also fails the consistency tests (not just automerge). We run those on all integration tests by default, so I need to either skip them or test that they fail. I chose to add a test-only dependency to MetaTesting.jl to be able to easily test that the consistency test suite fails when appropriate.~~

ericphanson avatar Jun 12 '24 19:06 ericphanson

ah ok looking into it more, it turns out I misunderstood the role of update_status. I thought it was an early exit so we don't run the rest of the guidelines. Instead it updates the commit status but continues running the rest of them.

In this case I wanted to early exit after checking isidentifier, since if that's not the case I'm not sure all the other checks will work as expected.

ericphanson avatar Jun 12 '24 20:06 ericphanson