Remove Spanish comments and align variable declarations - #130
Conversation
Eliminados comentarios en español de internal/http/handlers.go y router.go. Alineadas declaraciones de variables globales en handlers.go para mejorar legibilidad. Agregados parámetros de paginación (per_page, page) a funciones getTrendingGitHub, getTrendingGitLab y getTrendingCodeberg. Eliminado comentario HTML en README.md.
📝 WalkthroughWalkthroughThe change adds paginated trending requests, a popular repository tab, randomized repository display, and a combined legal page. It also removes explanatory comments across backend and frontend files and adds cleanup paths in temporary-directory tests. ChangesTrending pagination
Backend routing and cleanup
Frontend navigation and repository discovery
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/http/handlers.go`:
- Around line 3376-3383: The getTrendingGitLab language-backfill flow should
avoid unnecessary and excessive API calls for paginated results. Only initiate a
language request for projects whose Language is empty, and apply the same
backfill cap already used by searchGitLab; preserve existing behavior for
projects with populated languages and the surrounding pagination logic.
In `@internal/utils/temp_test.go`:
- Around line 75-81: The cleanup paths in the test must report failures from
both os.RemoveAll calls. Update the removal after the old-directory fallback and
the removal of recentDir in the Stat success branch to capture and check each
returned error, reporting failures through the test’s existing error mechanism
while preserving the current cleanup flow.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 44542971-60e7-464a-bec3-6f1e873f659b
⛔ Files ignored due to path filters (4)
web/assets/images/index-finger.svgis excluded by!**/*.svgweb/assets/images/surich.svgis excluded by!**/*.svgweb/assets/logos/gitgost-logo.svgis excluded by!**/*.svgweb/assets/sponsors/openbin.livrasand.com.pngis excluded by!**/*.png
📒 Files selected for processing (18)
README.mdinternal/http/handlers.gointernal/http/router.gointernal/http/router_test.gointernal/jobs/clone.gointernal/jobs/clone_test.gointernal/jobs/jobs.gointernal/jobs/retry.gointernal/jobs/run.gointernal/jobs/store.gointernal/jobs/store_test.gointernal/provider/codeberg/codeberg.gointernal/provider/github/github.gointernal/provider/gitlab/gitlab.gointernal/provider/provider.gointernal/utils/temp_test.goweb/index.htmlweb/repo.html
💤 Files with no reviewable changes (10)
- README.md
- internal/provider/github/github.go
- internal/jobs/clone_test.go
- internal/provider/provider.go
- internal/jobs/store.go
- internal/jobs/retry.go
- internal/provider/codeberg/codeberg.go
- internal/provider/gitlab/gitlab.go
- internal/jobs/clone.go
- internal/jobs/store_test.go
| func getTrendingGitLab(sort string, perPage, page int) []gin.H { | ||
| results := []gin.H{} | ||
|
|
||
| var url string | ||
| if sort == "new" { | ||
| url = "https://gitlab.com/api/v4/projects?order_by=created_at&sort=desc&per_page=10" | ||
| url = fmt.Sprintf("https://gitlab.com/api/v4/projects?order_by=created_at&sort=desc&per_page=%d&page=%d", perPage, page) | ||
| } else { | ||
| url = "https://gitlab.com/api/v4/projects?order_by=star_count&sort=desc&per_page=10" | ||
| url = fmt.Sprintf("https://gitlab.com/api/v4/projects?order_by=star_count&sort=desc&per_page=%d&page=%d", perPage, page) |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
Limit GitLab language backfills for paginated responses.
With per_page=100, getTrendingGitLab starts a language request for every returned project at Lines 3425-3437. The semaphore limits concurrency, but one request can still make 100 additional API calls and wait through ten timeout batches. loadPopular makes this request path routine.
Only backfill projects with an empty Language, and cap the number of backfills as searchGitLab already does.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/http/handlers.go` around lines 3376 - 3383, The getTrendingGitLab
language-backfill flow should avoid unnecessary and excessive API calls for
paginated results. Only initiate a language request for projects whose Language
is empty, and apply the same backfill cap already used by searchGitLab; preserve
existing behavior for projects with populated languages and the surrounding
pagination logic.
| os.RemoveAll(oldDir) | ||
| } | ||
|
|
||
| // Check that recent directory still exists | ||
| if _, err := os.Stat(recentDir); os.IsNotExist(err) { | ||
| t.Errorf("Recent directory should still exist: %s", recentDir) | ||
| } else { | ||
| os.RemoveAll(recentDir) // cleanup | ||
| os.RemoveAll(recentDir) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Check both os.RemoveAll errors.
The fallback at Line 75 and the cleanup at Line 81 ignore removal failures. A failed removal can leave temporary directories and affect later tests. Report each error.
Proposed fix
- os.RemoveAll(oldDir)
+ if err := os.RemoveAll(oldDir); err != nil {
+ t.Errorf("Failed to remove old directory: %v", err)
+ }
...
- os.RemoveAll(recentDir)
+ if err := os.RemoveAll(recentDir); err != nil {
+ t.Errorf("Failed to remove recent directory: %v", err)
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| os.RemoveAll(oldDir) | |
| } | |
| // Check that recent directory still exists | |
| if _, err := os.Stat(recentDir); os.IsNotExist(err) { | |
| t.Errorf("Recent directory should still exist: %s", recentDir) | |
| } else { | |
| os.RemoveAll(recentDir) // cleanup | |
| os.RemoveAll(recentDir) | |
| if err := os.RemoveAll(oldDir); err != nil { | |
| t.Errorf("Failed to remove old directory: %v", err) | |
| } | |
| } | |
| if _, err := os.Stat(recentDir); os.IsNotExist(err) { | |
| t.Errorf("Recent directory should still exist: %s", recentDir) | |
| } else { | |
| if err := os.RemoveAll(recentDir); err != nil { | |
| t.Errorf("Failed to remove recent directory: %v", err) | |
| } |
🧰 Tools
🪛 golangci-lint (2.12.2)
[error] 75-75: Error return value of os.RemoveAll is not checked
(errcheck)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/utils/temp_test.go` around lines 75 - 81, The cleanup paths in the
test must report failures from both os.RemoveAll calls. Update the removal after
the old-directory fallback and the removal of recentDir in the Stat success
branch to capture and check each returned error, reporting failures through the
test’s existing error mechanism while preserving the current cleanup flow.
Source: Linters/SAST tools
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a2fac4aa66
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| showSkeletons(); | ||
| const container = document.getElementById('repo-list'); | ||
| try { | ||
| const response = await fetch(`${API_BASE}/api/trending/all?sort=stars&per_page=100`); |
There was a problem hiding this comment.
When the new popular tab is opened, this requests sort=stars, but TrendingHandler only special-cases sort == "new"; every other value takes the existing GitHub trending path with a created:> cutoff for the last 30 days (internal/http/handlers.go:3307-3317). As a result, the popular tab excludes established high-star GitHub repositories while GitLab/Codeberg are sorted by all-time stars, so the cross-forge list is inconsistent. Either add a true stars mode without the GitHub recency cutoff or use one of the supported sorts.
Useful? React with 👍 / 👎.
2 participants
Eliminados comentarios en español de internal/http/handlers.go y router.go. Alineadas declaraciones de variables globales en handlers.go para mejorar legibilidad. Agregados parámetros de paginación (per_page, page) a funciones getTrendingGitHub, getTrendingGitLab y getTrendingCodeberg. Eliminado comentario HTML en README.md.
Summary by CodeRabbit
New Features
Improvements
Documentation