-
Notifications
You must be signed in to change notification settings - Fork 13
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
Removing uneeding branchName argument for github packages #238
Conversation
Once we merge this, I'll also merge on the server. |
If you want to test the api locally: https://github.com/b-rodrigues/git2nixsha/tree/main |
Tests pass locally, but complain here because when nix is not available, they try to reach the api, however the new version without branchName is not deployed it. I'd like your second opinion before pulling the trigger on the api and this PR. |
I agree its a good idea to do this now! |
Can we replace this: by the new approach like we do locally in Then we really have consistent behavior. To be sure, I think we should also add a new test (CI with nix installed), and compare equality of Overall, not using |
done with b-rodrigues/git2nixsha@01151eb |
After our discussions and some thought, I think that indeed, we should simply get rid of this argument, and better do it now before submitting to CRAN. I have also changed the code on the server, but not deployed yet; locally, it works!