automatic merging guidelines about general HOT 20 CLOSED

Yes, or people who just capitalize an entire package name for no reason, e.g. MYPACAKGE.jl.

from general.

We would only automerge PRs made by Registrator, which we control, not PRs made by people—those must be manually reviewed.

from general.

Personally, I would like to keep the automerging guidelines as conservative as possible.

The goal (at least as I envision it) is not to automerge every package, but rather to automerge many packages.

The use case that you describe is totally fine, but I think it’s okay to have those PR’s be manually merged.

from general.

Things I need for JLL package merging to be automatic:

• We need _ to be allowed in package names (because all JLL packages will end in _jll.jl)

• Non-standard version numbers; because the JLL package versions are linked to the upstream library version, they'll in general not be a nice number like 1.0.0, nor be nice, sequential versions.

My PRs currently come in under @staticfloat, but in the future I'll probably change it to be JuliaCIBot or something like that. Perhaps we can special-case PRs that end in _jll.jl so that they don't have the same version requirements?

from general.

We can hardcode some exceptions for JLL packages.

That being said, I would prefer that we first figure out the automerging for regular packages, get that merged, and make sure it works in production. Once automerging for regular packages has been deployed in production for a while, and we have fixed all of the bugs, then we can open a separate issue for automerging JLL packages? Does that sound like an okay plan?

from general.

@StefanKarpinski @fredrikekre What do you think about closing this issue?

Further issues regarding automerge can go here: https://github.com/JuliaRegistries/GeneralRegistryCI.jl/issues

from general.

ends in lowercase

? Many packages do not follow this, e.g. HTTP.

from general.

These guidelines are intended not as requirements for packages but as very conservative guidelines, which, if your new package or new version of a package meets them, it may be automatically merged.

A package with a name like HTTP would require review.

from general.

Is that to catch non-obvious acronyms or something?

from general.

This may go without saying but: We should also check every pull request to make sure that it only modifies files related to the package it claims to be related to.

For example, a pull request to register a new package Foo.jl would be allowed to create/modify files in the F/Foo directory, and would be allowed to add a line to Registry.toml containing the name, UUID, and path of Foo.jl, but would not be allowed to e.g. create/modify files in the B/Bar directory or modify lines in Registry.toml relating to the Bar.jl or Baz.jl packages. A pull request to register a new version for an existing package Foo.jl would be allowed to create/modify files in the F/Foo directory, but would not be allowed to modify Registry.toml or create/modify any other files.

Otherwise, we would have a security vulnerability:

• Suppose I maintain an existing package Foo.jl. The current version is v1.0.0.
• I submit a pull request to register a new version v2.0.0 of my package Foo.jl. So the "sequential version number" requirement is met ✅.
• I make sure that I have compat entries for all dependencies including Julia, and each compat entry has an upper bound ✅.
• Version v2.0.0 of Foo.jl is installable ✅ and loadable ✅ .
• Oh but also, by the way, my pull request changes the URL of OrderedCollections.jl to point to my fork of OrderedCollections.jl into which I have inserted some malicious code.

from general.

We would only automerge PRs made by Registrator, which we control, not PRs made by people—those must be manually reviewed.

Perfect!

from general.

I have opened a pull request that implements these automatic merging guidelines: #3342

from general.

Compat for all dependencies

• all [deps] should also have [compat] entries (and Julia itself)
• [compat] entries should have upper bounds

I would like to suggest ammending this to:

Compat for all changed/added dependencies

• all new [deps] should also have [compat] entries (and Julia itself)
• modified [compat] entries should have upper bounds

Basically, if you didn't touch a dep or its compat, then you should still get auto-merge.
I am a big fan of caret bounding everything,
but exceptions can apply.

For example lets say my package depends on DataFrames.jl,
but the only thing I actually depend upon is that DataFrames exports the AbstractDataFrame type, because I have some type-constraint related to that.
E.g. I just define a trait obsdim(::Type{AbstractDataFrame})=2).

So I only lower-bound it, because I am pretty confidant that DataFrames is not going to stop exporting that type.
Then when I tag a release for the first time, it is blocked from automerge because of this,
and I explain to the General registry maintainer why I am only lower-bounding it.
And they are like "Yeah, makes sense. [merge]".

Now later I update my package, I haven't changed how I use DataFrames,
I don't want to be blocked and having to reexplain each time why.

But If later I do bump my minimum DataFrames compat, then I should be stopped since my past evidence has been proven wrong, and I do need the upper-bounds.
and also because I need to go an ret-con the registry to say that all my old version also should be upper bounded.

from general.

@oxinabox The other thing we can do in your specific use case is set the compat for DataFrames to something like 0.18.0 <= x < 1.0.0

from general.

The use case that you describe is totally fine, but I think it’s okay to have those PR’s be manually merged.

Manually merged the first time.
But after the 15th?
Not so much.

It is fine for packages on slow release cycles,
but when practicing continous delivery of packages with dependencies being concurently devleoped,
it isn't unreasonable to release every day for a week.

@oxinabox The other thing we can do in your specific use case is set the compat for DataFrames to something like 0.18.0 <= x < 1.0.0

That is not a legal compat specifier, AFAIK.

Could say: <=100.0 but that means not allowe a lower bound.
Alos: I think we should ideally not automerge things like that too, since it is probably a typo.

from general.

That is not a legal compat specifier, AFAIK.

It will be at some point. See JuliaLang/Pkg.jl#1232

from general.

Also, to make life a little easier, would it be possible for you to use the same title format as Registrator for your pull request?

For example, Registrator pull request titles look like this:

• New package: Foo v1.2.3
• New version: Foo v1.2.3

Could you change the format of your titles to look like this:

• New package: Ncurses_jll v6.1.0+0
• New version: Ncurses_jll v6.1.0+0

Do you think that would be possible? It would make my life much easier when I start working on automerging for JLL packages?

from general.

Yes; is it strictly necessary to disambiguate New package vs. New version? Especially since the version requirements for a new JLL package will not be stricter, as they are for normal packages?

from general.

Presumably we will keep the existing requirement that new packages have a 3 day waiting period.

from general.

@staticfloat I opened an issue specifically dedicated to the automerging of JLL packages: JuliaRegistries/RegistryCI.jl#6

from general.