-
Notifications
You must be signed in to change notification settings - Fork 749
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
use git clone instead of github api #1006
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
does this work with local folders? this won't work if user does not have git |
Added local folder support. |
well im sure this code will break in prod using git to just download a zip seems overkill is there a way to download the repo using request without rate limit? also to take into account, in the future, we will have to download repo from other users, not monorepo idk how obsidian does it https://github.com/obsidianmd/obsidian-releases (they have a store) |
git exists solely for that purpose alone and it's the most efficient way, the code uses sparse checkout to only download the specified path.
It will work fine with any repo it's literally just git.
problem isn't downloading a zip, it's the way git protocol works it's pretty complex and requires the spec file for it to know which files to download, that's why git cli is a good approach for this. any other approach will have lot of complexity, maintenance and cost. |
Looked into Obsidian approach they use GitHub releases to distribute the plugins, this requires lot of extra steps from developers It fetches version from manifest.json |
so its a bit like the ffmpeg-sidecar now, installing git at runtime? why not we could also package git at build time in the files? maybe better - should have to change the install.sh, install.ps1 (CLI only) and pre_build.js (app+cli) i think atm the pswh script have to be packaged as file so it's annoying let's use git then |
That's a good idea, I forgot we have installation scripts, will move the logic there. |
also alternatively https://github.com/rust-lang/git2-rs @neo773 but might need some lib file too idk |
Pushed a new commit |
seems you can remove the runtime installation of git in the code? also keep in mind we use pre_build.js for the app build |
f573cdb
to
0ea2a0a
Compare
whats the status? can you remove runtime installation of git? will cause issues |
This PR removes usage of GitHub API which ran into rate limiting issues and instead uses the powerful git cli to clone only the specified path so it's highly efficient and avoids all rate limits.
From my testing it seems to be working.
b.mp4
/claim #905
/closes #905