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

Fix add_general_registry #28

Merged
merged 13 commits into from
Dec 20, 2022
Merged

Conversation

IanButterworth
Copy link
Member

Fixes #27

Need to look into testing this. Evidently it wasn't tested in #25

@IanButterworth IanButterworth requested a review from a team as a code owner December 19, 2022 20:02
@IanButterworth
Copy link
Member Author

IanButterworth commented Dec 19, 2022

@SaschaMann @DilumAluthge I haven't figured out why this wasn't detected in tests here, given that julia-buildpkg should be calling add_general_registry here too.. but perhaps worth fixing this quick then figuring out the test later

@DilumAluthge
Copy link
Member

Presumably the isfile check below should check for either General/Registry.toml or General.toml?

@codecov-commenter
Copy link

codecov-commenter commented Dec 19, 2022

Codecov Report

Base: 100.00% // Head: 100.00% // No change to project coverage 👍

Coverage data is based on head (1191dbc) compared to base (f813042).
Patch has no changes to coverable lines.

❗ Current head 1191dbc differs from pull request most recent head 9355049. Consider uploading reports for the commit 9355049 to get more accurate results

Additional details and impacted files
@@            Coverage Diff            @@
##            master       #28   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            1         1           
  Lines            2         2           
=========================================
  Hits             2         2           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@IanButterworth
Copy link
Member Author

IanButterworth commented Dec 19, 2022

Something fishy is going on with the CI set up here.

Prior to 5070133 (#28) it seemed like the add_general_registry.jl file was being overwritten by the one in julia-runtest during step install.

However 5070133 (#28) apparently fixed that, but only by adding a missing do.

I think it was just a race between which julia- step got installed first.

Renaming that file is the robust fix though, both here and julia-runtest

@SaschaMann
Copy link
Member

As mentioned in julia-runtest, I don't understand why this would be necessary, GITHUB_ACTION_PATH points to the repo of the action, not some shared workspace. They shouldn't interfere with each other in any way. How does this cause issues?

@SaschaMann SaschaMann merged commit 252d468 into julia-actions:master Dec 20, 2022
@SaschaMann
Copy link
Member

I'll merge it already to fix the bug but I'm really unsure what happened there and would like to figure that out :D

@IanButterworth
Copy link
Member Author

If the overwrite theory is false I don't have a clue 😬

@IanButterworth IanButterworth deleted the patch-2 branch December 20, 2022 14:50
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.

Error cloning General registry using buildpkg v1.3.1
4 participants