chore: align the source of the list of available providers with the source of providers#528
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughUpdated the provider list command to fetch repositories from the GitHub user Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
cmd/provider/list-default-providers.go (1)
22-22: Extract the GitHub owner into a shared constant to prevent future drift.The owner (
skevetter) is now duplicated across the API URL and printed label. Centralizing it keeps these values aligned going forward.♻️ Proposed refactor
+const availableProvidersGitHubOwner = "skevetter" + func getDevpodProviderList() error { - req, err := http.NewRequest("GET", "https://api.github.com/users/skevetter/repos?per_page=100", nil) + req, err := http.NewRequest("GET", fmt.Sprintf("https://api.github.com/users/%s/repos?per_page=100", availableProvidersGitHubOwner), nil) if err != nil { return err } @@ - fmt.Println("List of available providers from skevetter:") + fmt.Printf("List of available providers from %s:\n", availableProvidersGitHubOwner)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/provider/list-default-providers.go` at line 22, Extract the GitHub owner string into a single shared constant (e.g., Owner or githubOwner) and use that constant wherever the owner is used instead of the literal "skevetter"; update the http.NewRequest call that builds the URL (the line with req, err := http.NewRequest("GET", "https://api.github.com/users/skevetter/repos?per_page=100", nil)) to format the URL using the constant and also replace any printed labels or logging that currently embed "skevetter" so they reference the same constant (ensuring consistent owner value across the request and output).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@cmd/provider/list-default-providers.go`:
- Line 22: Extract the GitHub owner string into a single shared constant (e.g.,
Owner or githubOwner) and use that constant wherever the owner is used instead
of the literal "skevetter"; update the http.NewRequest call that builds the URL
(the line with req, err := http.NewRequest("GET",
"https://api.github.com/users/skevetter/repos?per_page=100", nil)) to format the
URL using the constant and also replace any printed labels or logging that
currently embed "skevetter" so they reference the same constant (ensuring
consistent owner value across the request and output).
|
With this change, the list of available providers becomes shorter, since not all Loft providers are currently forked (and released) into the community fork; if some of the missing providers are, as I suspect, needed, their repositories need to be forked and set up, and releases made; missing are:
|
|
I'm going to wait to first fork the remaining providers before merging. |
Why? Remaining providers are already not installable, since |
Valid. I was not aware of the current situation. Thank you for pointing that out. |
5eb81fa to
0edf4f8
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cmd/provider/list-default-providers.go (1)
3-14:⚠️ Potential issue | 🔴 CriticalAdd missing
osimport and fix invalidos.STDOUTsymbol.Line 47 uses
os.STDOUT, butosis not imported. Additionally,os.STDOUTis not a valid Go symbol; the correct symbol isos.Stdout. This will fail to compile.Suggested fix
import ( "context" "encoding/json" "fmt" "io" "net/http" + "os" "strings" @@ - fmt.Fprintln(os.STDOUT, "\t", name) + fmt.Fprintln(os.Stdout, "\t", name)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/provider/list-default-providers.go` around lines 3 - 14, The code references os.STDOUT but doesn't import os and uses the wrong symbol; update the import block to include "os" and replace any use of os.STDOUT with the correct os.Stdout (e.g., in the function that writes JSON or uses fmt.Fprint to output to os.Stdout), ensuring all references (os.STDOUT -> os.Stdout) and imports are corrected so the code compiles.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@cmd/provider/list-default-providers.go`:
- Around line 3-14: The code references os.STDOUT but doesn't import os and uses
the wrong symbol; update the import block to include "os" and replace any use of
os.STDOUT with the correct os.Stdout (e.g., in the function that writes JSON or
uses fmt.Fprint to output to os.Stdout), ensuring all references (os.STDOUT ->
os.Stdout) and imports are corrected so the code compiles.
1e6b7ce to
36235a2
Compare
…ource of providers Signed-off-by: Leonid Dubinsky <dub@podval.org>
36235a2 to
e81d500
Compare
Summary by CodeRabbit