Copying a discussion from RegistryHealthChecks.jl #36
Description
Rename package from RegistryCIHealthChecks.jl to RegistryHealthChecks.jl #5
Merged
DilumAluthge merged 1 commit into main from dpa/rename 10 days ago
Merged
Rename package from RegistryCIHealthChecks.jl to RegistryHealthChecks.jl
#5
DilumAluthge merged 1 commit into main from dpa/rename 10 days ago
+23 −23
Conversation 10
Commits 1
Checks 4
Files changed 8
Conversation
@DilumAluthge
Member
DilumAluthge commented 10 days ago
No description provided.
@DilumAluthge
Rename package from RegistryCIHealthChecks.jl to `RegistryHealthChe… …
Verified
36599bd
Revert
View details @DilumAluthge DilumAluthge merged commit 454ad6c into main 10 days ago
4 checks passed
Restore branch
@DilumAluthge DilumAluthge deleted the dpa/rename branch 10 days ago
@KristofferC
Member
KristofferC commented 10 days ago
Out of curiosity, what is the "health of the registry" and how is it checked? Is it if there are any problems with the entries in the registry that can cause errors on Pkg operations? Isn't the integrity of the registry already checked in its CI?
@DilumAluthge
Member
Author
DilumAluthge commented 10 days ago
The idea is to do stuff like this: #30
Maybe RegistryHealthChecks isn't the best name? Would PackageHealthChecks be more informative?
In theory we could just put these checks into RegistryCI.jl. But these are checks that wouldn't necessarily block AutoMerge, so they don't need to go in RegistryCI.jl.
@KristofferC
Member
KristofferC commented 10 days ago
I guess calling it RegistryHealth makes you think it is about the health of the registry but the core here doesn't really have anything to do with the registry itself. It seems to be more akin to a "best practice linter" that you can run on a package. Sure, you could loop that check over all packages in the registry but that is a by-product. You can just as well run it on a private package that isn't registered.
👍 2
@DilumAluthge
Member
Author
DilumAluthge commented 10 days ago
Maybe PackageBestPractices.jl?
@KristofferC
Member
KristofferC commented 9 days ago
Something along that line seems reasonable to me at least.
@DilumAluthge
Member
Author
DilumAluthge commented 9 days ago
See also #4
@ericphanson
Member
ericphanson commented 9 days ago
I like something with "best practices" in the name, but also I think AnalyzeRegistry might be the right package for this functionality to live in! cc @giordano. In #20 we're bikeshedding the name and API for that package (i.e. it might not stay called "AnalyzeRegistry"). AnalyzeRegistry doesn't have a ton to do with the registry except it provides an easy way to clone a bunch of packages from the registry and populate their repo URLs and subdir settings from the registry. Much of the functionality is per-package. Already you can do using AnalyzeRegistry; analyze(".") to analyze code you are developing.
I am interested in working on functionality like #30 mostly from a best-practices angle instead of a compliance angle. I.e. I think it would be really great to have tooling so new dev's can analyze/healthcheck/best-practice-check/whatever their code and get a little checkbox list or something showing what it's found and what isn't there yet. And then I think it could have a bunch of links to pedagodical material about things like "what is CI?", "what are licenses?" etc. (ideally in a way that's not super verbose/cluttering). (I'm also interested in working on AnalyzeRegistry from an registry-wide stats/health point of view, but that's another topic).
However, I think a version of this could also be commented in General PRs so that package authors are shown these things and get some more exposure to best-practices. I think it could also show stats and info about the package that isn't purely best practices, like showing LoC counts. I think that would just be kind of interesting to see as a package author. But of course whether or not it goes in comments on General PRs depends on what the community thinks, e.g. is it annoying.
I think this could also check for project-key/license-file mismatches like we talked about on the pkg-call (even though that dips towards the compliance direction), since it might be a reasonable place to place some non-fatal errors (and of course it's is a best practice to not have mismatches like that!).
👍 1
@giordano
Member
giordano commented 9 days ago
Yes, we'll very likely rename the package AnalyzeRegistry: as Eric says, there are a couple of utilities to run the checks on all packages in a registry, but the core functions just inspect a package directory.
I think it would be really great to have tooling so new dev's can analyze/healthcheck/best-practice-check/whatever their code and get a little checkbox list or something showing what it's found and what isn't there yet. And then I think it could have a bunch of links to pedagodical material about things like "what is CI?", "what are licenses?" etc. (ideally in a way that's not super verbose/cluttering).
While I agree this is a cool idea to promote best practices, the more I think of it the more I'm not sure adding more material to AutoMerge messages is useful: there is already quite some noise and 90% of the developers don't read at all messages from AutoMerge. And I'm sure many of the developers who'll actually read these messages won't appreciate/care about this.
Perhaps some sort of GitHub Actions workflow which shows a report of the package, for developers who opt-in, can be more useful (I think Dilum suggested it during the Pkg call on Tuesday).
👍 3
@ericphanson
Member
ericphanson commented 28 minutes ago •
edited
new idea: some info could be added to the Registrator comment, instead of the automerge comment. For example, taking a recent Registrator comment, we could add three lines to it:
Registering package: AbstractAlgebra
Repository: https://github.com/Nemocas/AbstractAlgebra.jl
Created by: @thofma
Version: v0.15.1
Commit: e2e7fd83ff4440248666a35256ff010c72c9b293
Reviewed by: @thofma
Reference: Nemocas/AbstractAlgebra.jl@e2e7fd8#commitcomment-48494446
lines of Julia code: 30199 in src, 13235 in test
license(s): BSD-2-Clause (in LICENSE.md)
CI ✓, documentation ✓, tests ✓
which I think provides some interesting info without adding much noise. This could be done by exposing a function in AnalyzeRegistry to provide this info formatted in a convenient way and PRing Registrator to add it to the comment.
(I mentioned a version of this to @giordano already and he seemed onboard)
(edit: sorry for pinging you @thofma, just grabbed the text from a recent General PR as an example here and forgot that it would ping you).