-
Notifications
You must be signed in to change notification settings - Fork 479
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Contributing Guidelines] Add script to check that versions are present #75328
Conversation
This can help with PRs like #75319
julia> check_package_versions("FastParzenWindows", "https://github.com/ngiann/FastParzenWindows.jl.git");
Cloning into '/var/folders/v2/hmy3kzgj4tb3xsy8qkltxd0r0000gn/T/jl_CBJohl'...
remote: Enumerating objects: 261, done.
remote: Counting objects: 100% (79/79), done.
remote: Compressing objects: 100% (68/68), done.
remote: Total 261 (delta 40), reused 26 (delta 10), pack-reused 182
Receiving objects: 100% (261/261), 98.03 KiB | 760.00 KiB/s, done.
Resolving deltas: 100% (142/142), done.
ERROR: UndefVarError: `pretty_print` not defined
Stacktrace:
[1] check_packages_versions(pkg_names::Vector{String}, repo_url::String; registry_uuid::UUID, verbose::Bool, throw::Bool)
@ Main /tmp/foo.jl:33
[2] check_packages_versions
@ /tmp/foo.jl:15 [inlined]
[3] #check_package_versions#8
@ /tmp/foo.jl:38 [inlined]
[4] check_package_versions(pkg_name::String, repo_url::String)
@ Main /tmp/foo.jl:38
[5] top-level scope
@ REPL[8]:1 |
Co-authored-by: Mosè Giordano <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I confirm it works well now, thanks, this is very useful to add to the README!
Side question: Is there any package where this snippet could live, to make this easier to test instead of copying a bunch of functions? But this can be done as a follow up.
I’m not sure. It would be nice to somehow automate this check, maybe as another workflow which runs on all PRs and detects URL changes. Maybe with the code in RegistryCI? And another workflow so it can get triggered on manual PRs. |
Oops, clicked the wrong button. I think it would make sense to put the code in RegistryCI. It wouldn’t be part of automerge but I think it could make sense to put it in the package anyway. Then also we could later try to add another workflow to run the code on PRs with URL changes. (edit: I see my former reply was also submitted when I accidentally closed the PR so now I rewrote this one w the same stuff aha). |
Yes, that'd be cool |
Let’s put it here for now though and update it if it gets moved somewhere better. |
This can help with PRs like #75319
Based on @GunnarFarneback's script from #35965 (comment), but updated to use RegistryInstances many folks don't have git registries. (Also should be usable for other registries by changing the
registry_uuid
; not super relevant here but I know folks often copy-paste stuff from General in order to setup other registries).