Skip to content
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

Merged
merged 3 commits into from
Jan 8, 2023

Conversation

ericphanson
Copy link
Member

@ericphanson ericphanson commented Jan 8, 2023

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).

@ericphanson ericphanson temporarily deployed to stopwatch January 8, 2023 15:14 — with GitHub Actions Inactive
@ericphanson ericphanson requested a review from giordano January 8, 2023 15:50
@giordano
Copy link
Member

giordano commented Jan 8, 2023

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

CONTRIBUTING.md Outdated Show resolved Hide resolved
Co-authored-by: Mosè Giordano <[email protected]>
@ericphanson ericphanson temporarily deployed to stopwatch January 8, 2023 17:10 — with GitHub Actions Inactive
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
@ericphanson ericphanson temporarily deployed to stopwatch January 8, 2023 17:14 — with GitHub Actions Inactive
Copy link
Member

@giordano giordano left a 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.

@ericphanson
Copy link
Member Author

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.

@ericphanson ericphanson closed this Jan 8, 2023
@ericphanson ericphanson reopened this Jan 8, 2023
@ericphanson ericphanson temporarily deployed to stopwatch January 8, 2023 21:56 — with GitHub Actions Inactive
@ericphanson
Copy link
Member Author

ericphanson commented Jan 8, 2023

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).

@giordano
Copy link
Member

giordano commented Jan 8, 2023

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?

Yes, that'd be cool

@ericphanson
Copy link
Member Author

ericphanson commented Jan 8, 2023

Let’s put it here for now though and update it if it gets moved somewhere better.

@ericphanson ericphanson merged commit 9a406ba into master Jan 8, 2023
@ericphanson ericphanson deleted the eph/add-version-check-script branch January 8, 2023 22:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants