Closed ericphanson closed 2 months ago
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.
I think this is pretty simple and uncontroversial so I'll merge soon
I haven't reviewed the code, but your description sounds like this is the right approach.
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.
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.
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.