-
Notifications
You must be signed in to change notification settings - Fork 5
Remove Turkish comments and localize theme labels to English #88
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
Remove Turkish comments and localize theme labels to English #88
Conversation
Introduce a Makefile for Docker and development shortcuts, add VSCode settings for Makefile integration, and update the README with detailed Docker and development instructions. Update the GitHub API client for improved token handling, error management, and code style consistency. Adjust the dev script in package.json to use the correct Next.js CLI flag and host binding.
Turkish comments were removed from ThemeProvider for clarity. Theme labels and mode names in ThemeToggle were changed from Turkish to English to improve localization and consistency.
WalkthroughInfrastructure, build, and documentation updates across the project: VSCode workspace settings enable Makefile configuration, a comprehensive Makefile introduces Docker and development workflows, README expanded with setup and Docker-based guidance, package.json updates the dev server to use Turbo with external accessibility, theme component labels translated from Turkish to English, and the API client interface adopts consistent formatting. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes The changes span infrastructure setup, documentation, build configuration, UI translation, and code formatting across multiple file types. While individual edits in some areas are straightforward (label translation, formatting), the breadth of heterogeneous changes—from Makefile Docker targets to README reorganization to interface styling—requires separate reasoning for each cohort and context-switching between domains. Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (6)
Makefile (1)
8-37: Makefile style warnings from checkmake are acceptable for this project.Static analysis flagged that
helpandenvtarget bodies exceed the recommended maximum length and that standard phony targets (all,clean,test) are missing. These are style conventions that don't apply to this project:
- The
helptarget intentionally provides comprehensive documentation.- The
envtarget contains legitimate multi-statement logic.- The project uses
clean-dockerinstead of a genericcleantarget, which is contextually appropriate.These warnings can be safely ignored or suppressed via a
.checkmake.ymlconfiguration file if needed.Also applies to: 44-50
README.md (1)
227-257: Minor markdown style improvement.Lines 237 and 243 use bold emphasis (
**Option 1: ...**) instead of proper markdown headings (### Option 1: ...). Converting these to subheadings would improve document structure and make the outline more navigable. This is a low-priority style refinement.src/components/theme/ThemeToggle.tsx (1)
48-49: Leftover Turkish placeholder.Translate placeholder to English to complete localization.
Apply:
- <SelectValue placeholder="Tema seçin" /> + <SelectValue placeholder="Select theme" />src/lib/api/github-api-client.ts (3)
7-11: Duplicate GitHubSearchResponse type.This redefines a type already exported at src/types/api.ts. Prefer importing to avoid drift.
Apply:
-import type { - GitHubUserDetailed, - GitHubRepositoryDetailed, -} from "@/types/github"; +import type { + GitHubUserDetailed, + GitHubRepositoryDetailed, +} from "@/types/github"; +import type { GitHubSearchResponse } from "@/types/api"; -interface GitHubSearchResponse<T> { - items: T[]; - total_count: number; - incomplete_results: boolean; -}
790-794: Tighten typing on fetchWithCache usage.Add a generic to preserve type safety.
Apply:
- const endpoint = `/users/${username}`; - return await this.fetchWithCache(endpoint, true); + const endpoint = `/users/${username}`; + return await this.fetchWithCache<GitHubUserDetailed>(endpoint, true);
416-449: Return types areunknown[]; consider explicit shapes.These methods build consistent objects (e.g., id/title/repo/type/priority/labels/...). Strong typing will catch regressions and improve DX.
- Define interfaces: AssignedItem, MentionItem, ReviewRequestItem, StaleItem.
- Update signatures to return those arrays instead of unknown[].
Also applies to: 452-514, 516-548, 550-584, 586-636
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.vscode/settings.json(1 hunks)Makefile(1 hunks)README.md(4 hunks)package.json(1 hunks)src/components/theme/ThemeProvider.tsx(0 hunks)src/components/theme/ThemeToggle.tsx(2 hunks)src/lib/api/github-api-client.ts(11 hunks)
💤 Files with no reviewable changes (1)
- src/components/theme/ThemeProvider.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
src/lib/api/github-api-client.ts (4)
src/types/api.ts (1)
GitHubSearchResponse(126-130)src/lib/api/github-graphql-client.ts (1)
query(157-199)src/types/oss-insight.ts (2)
TrendingRepo(1-36)TopContributor(69-87)src/types/github.ts (2)
GitHubUserDetailed(73-106)GitHubRepositoryDetailed(108-195)
🪛 checkmake (0.2.2)
Makefile
[warning] 8-8: Target body for "help" exceeds allowed length of 5 (28).
(maxbodylength)
[warning] 44-44: Target body for "env" exceeds allowed length of 5 (6).
(maxbodylength)
[warning] 1-1: Missing required phony target "all"
(minphony)
[warning] 1-1: Missing required phony target "clean"
(minphony)
[warning] 1-1: Missing required phony target "test"
(minphony)
[warning] 101-101: Target "dev" should be declared PHONY.
(phonydeclared)
[warning] 102-102: Target "prod" should be declared PHONY.
(phonydeclared)
[warning] 103-103: Target "up" should be declared PHONY.
(phonydeclared)
[warning] 104-104: Target "down" should be declared PHONY.
(phonydeclared)
[warning] 105-105: Target "logs" should be declared PHONY.
(phonydeclared)
[warning] 106-106: Target "restart" should be declared PHONY.
(phonydeclared)
🪛 LanguageTool
README.md
[style] ~216-~216: It’s more common nowadays to write this noun as one word.
Context: ...ely ## Settings - Change organization/user name and token anytime from the Settings pag...
(RECOMMENDED_COMPOUNDS)
🪛 markdownlint-cli2 (0.18.1)
README.md
237-237: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
243-243: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🔇 Additional comments (10)
package.json (1)
6-6: Dev server now binds to all network interfaces.The change from
--turbopackto--turbo -H 0.0.0.0enables external accessibility needed for Docker development workflows. However, binding to0.0.0.0exposes the development server to the network without authentication.Ensure this is documented and only used in development environments. Consider adding a note to the README or
.env.examplewarning against this in production or untrusted networks..vscode/settings.json (1)
1-3: LGTM!This VSCode setting enables automatic Makefile configuration, improving developer ergonomics when using the new Make-based workflows introduced in this PR.
Makefile (1)
1-106: Well-organized Makefile with good developer ergonomics.The Makefile is well-structured with clear sections, consistent variable usage, and helpful status messages. The Docker workflow targets align well with the documented setup in README.md and the new development paradigm introduced in this PR.
README.md (5)
22-23: Optional prerequisites well-positioned.The new optional prerequisites for Make and Docker are clearly marked as optional, helping users choose their preferred setup path. Good UX improvement.
27-69: Setup sections are clear and well-structured.Both the Standard Setup (Local Development) and Docker Setup (Alternative) paths are well-documented with numbered steps, code blocks, and clear visual separation. Users can easily choose their preferred approach.
116-202: Excellent development workflow documentation.The Development, Docker Development, and Docker Production sections provide comprehensive, parallel documentation for Make-based and direct docker-compose workflows. The Make Commands Reference table is well-formatted and complete.
227-278: Platform-specific and workflow guidance is practical and well-organized.The Platform-Specific Notes section provides clear pathways for Windows, Linux, and Mac users, and the Development Workflow Recommendations section offers scenario-driven guidance that aligns with common use cases (solo prototyping vs. team production-ready testing).
1-286: Content is internally consistent and well-coordinated with other files.The README accurately references all Makefile targets, npm scripts, and environment setup steps. The expansion is comprehensive, well-organized, and provides clear pathways for different developer preferences and scenarios.
src/components/theme/ThemeToggle.tsx (2)
54-55: Localization updates look good.Light/Dark/System labels correctly translated to English. No behavioral changes.
Also applies to: 60-61, 66-67
86-87: Minimal toggle labels are correct.“Dark Mode” / “Light Mode” read well and match icon states.
Also applies to: 91-92
| @@ -0,0 +1,106 @@ | |||
| .PHONY: help setup env docker-dev docker-dev-build docker-dev-logs docker-dev-down docker-prod docker-up docker-down docker-logs docker-restart clean-docker | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add quick aliases to .PHONY declaration.
The quick alias targets (dev, prod, up, down, logs, restart) on lines 101–106 are not declared in the .PHONY declaration on line 1. Without this, make will treat them as file targets, potentially causing issues if files with these names exist.
Apply this diff to add the aliases to the .PHONY declaration:
-.PHONY: help setup env docker-dev docker-dev-build docker-dev-logs docker-dev-down docker-prod docker-up docker-down docker-logs docker-restart clean-docker
+.PHONY: help setup env docker-dev docker-dev-build docker-dev-logs docker-dev-down docker-prod docker-up docker-down docker-logs docker-restart clean-docker dev prod up down logs restartAlso applies to: 101-106
🧰 Tools
🪛 checkmake (0.2.2)
[warning] 1-1: Missing required phony target "all"
(minphony)
[warning] 1-1: Missing required phony target "clean"
(minphony)
[warning] 1-1: Missing required phony target "test"
(minphony)
🤖 Prompt for AI Agents
In Makefile at line 1 (and referencing quick alias targets on lines 101–106),
the .PHONY declaration is missing the short alias targets (dev, prod, up, down,
logs, restart); update the .PHONY line to include those aliases so make treats
them as phony targets (e.g., add dev prod up down logs restart to the
comma/space-separated list on line 1).
| const isClassicToken = /^ghp_[A-Za-z0-9]{36}$/.test(trimmedToken); | ||
| const isFineGrainedToken = /^github_pat_[A-Za-z0-9_]{82}$/.test( | ||
| trimmedToken | ||
| ); | ||
| const isGitHubAppToken = /^ghs_[A-Za-z0-9]{36}$/.test(trimmedToken); | ||
| const isLegacyToken = /^[a-f0-9]{40}$/.test(trimmedToken); | ||
|
|
||
| const isOAuthToken = | ||
| /^gho_[A-Za-z0-9_-]{16,}$/.test(trimmedToken) || | ||
| (!isClassicToken && | ||
| !isFineGrainedToken && | ||
| !isGitHubAppToken && | ||
| !isLegacyToken && | ||
| /^[A-Za-z0-9_-]{20,255}$/.test(trimmedToken)); | ||
|
|
||
| if ( | ||
| !isClassicToken && | ||
| !isFineGrainedToken && | ||
| !isGitHubAppToken && | ||
| !isLegacyToken && | ||
| !isOAuthToken | ||
| ) { | ||
| throw new Error( | ||
| 'Invalid GitHub token format. Expected:\n' + | ||
| '- Classic token: ghp_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx (40 chars)\n' + | ||
| '- Fine-grained token: github_pat_xxxxxxxxxx... (94 chars)\n' + | ||
| '- GitHub App token: ghs_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx (40 chars)\n' + | ||
| '- OAuth token: 20-255 character alphanumeric string\n' + | ||
| '- Legacy token: 40 character hexadecimal string' | ||
| ) | ||
| "Invalid GitHub token format. Expected:\n" + | ||
| "- Classic token: ghp_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx (40 chars)\n" + | ||
| "- Fine-grained token: github_pat_xxxxxxxxxx... (94 chars)\n" + | ||
| "- GitHub App token: ghs_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx (40 chars)\n" + | ||
| "- OAuth token: 20-255 character alphanumeric string\n" + | ||
| "- Legacy token: 40 character hexadecimal string" | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Over‑strict token regex + mismatch with error text (rejects valid tokens).
- Fine‑grained token regex enforces exact length 82, but lengths vary; error message claims 94 chars. This can block valid tokens.
- Better: validate by prefix/pattern and rely on length checks below (40–255).
Use a permissive pattern and fix the message:
-const isFineGrainedToken = /^github_pat_[A-Za-z0-9_]{82}$/.test(trimmedToken);
+const isFineGrainedToken = /^github_pat_[A-Za-z0-9_]+$/.test(trimmedToken);
if (
!isClassicToken &&
!isFineGrainedToken &&
!isGitHubAppToken &&
!isLegacyToken &&
!isOAuthToken
) {
throw new Error(
- "Invalid GitHub token format. Expected:\n" +
- "- Classic token: ghp_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx (40 chars)\n" +
- "- Fine-grained token: github_pat_xxxxxxxxxx... (94 chars)\n" +
- "- GitHub App token: ghs_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx (40 chars)\n" +
- "- OAuth token: 20-255 character alphanumeric string\n" +
- "- Legacy token: 40 character hexadecimal string"
+ "Invalid GitHub token format. Expected:\n" +
+ "- Classic token: ghp_...\n" +
+ "- Fine-grained token: github_pat_...\n" +
+ "- GitHub App token: ghs_...\n" +
+ "- OAuth token: 20–255 character string\n" +
+ "- Legacy token: 40 character hexadecimal string"
);
}Rationale: we already enforce min/max length at Lines 160–170; prefix checks suffice while avoiding false negatives.
📝 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.
| const isClassicToken = /^ghp_[A-Za-z0-9]{36}$/.test(trimmedToken); | |
| const isFineGrainedToken = /^github_pat_[A-Za-z0-9_]{82}$/.test( | |
| trimmedToken | |
| ); | |
| const isGitHubAppToken = /^ghs_[A-Za-z0-9]{36}$/.test(trimmedToken); | |
| const isLegacyToken = /^[a-f0-9]{40}$/.test(trimmedToken); | |
| const isOAuthToken = | |
| /^gho_[A-Za-z0-9_-]{16,}$/.test(trimmedToken) || | |
| (!isClassicToken && | |
| !isFineGrainedToken && | |
| !isGitHubAppToken && | |
| !isLegacyToken && | |
| /^[A-Za-z0-9_-]{20,255}$/.test(trimmedToken)); | |
| if ( | |
| !isClassicToken && | |
| !isFineGrainedToken && | |
| !isGitHubAppToken && | |
| !isLegacyToken && | |
| !isOAuthToken | |
| ) { | |
| throw new Error( | |
| 'Invalid GitHub token format. Expected:\n' + | |
| '- Classic token: ghp_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx (40 chars)\n' + | |
| '- Fine-grained token: github_pat_xxxxxxxxxx... (94 chars)\n' + | |
| '- GitHub App token: ghs_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx (40 chars)\n' + | |
| '- OAuth token: 20-255 character alphanumeric string\n' + | |
| '- Legacy token: 40 character hexadecimal string' | |
| ) | |
| "Invalid GitHub token format. Expected:\n" + | |
| "- Classic token: ghp_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx (40 chars)\n" + | |
| "- Fine-grained token: github_pat_xxxxxxxxxx... (94 chars)\n" + | |
| "- GitHub App token: ghs_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx (40 chars)\n" + | |
| "- OAuth token: 20-255 character alphanumeric string\n" + | |
| "- Legacy token: 40 character hexadecimal string" | |
| ); | |
| const isClassicToken = /^ghp_[A-Za-z0-9]{36}$/.test(trimmedToken); | |
| const isFineGrainedToken = /^github_pat_[A-Za-z0-9_]+$/.test(trimmedToken); | |
| const isGitHubAppToken = /^ghs_[A-Za-z0-9]{36}$/.test(trimmedToken); | |
| const isLegacyToken = /^[a-f0-9]{40}$/.test(trimmedToken); | |
| const isOAuthToken = | |
| /^gho_[A-Za-z0-9_-]{16,}$/.test(trimmedToken) || | |
| (!isClassicToken && | |
| !isFineGrainedToken && | |
| !isGitHubAppToken && | |
| !isLegacyToken && | |
| /^[A-Za-z0-9_-]{20,255}$/.test(trimmedToken)); | |
| if ( | |
| !isClassicToken && | |
| !isFineGrainedToken && | |
| !isGitHubAppToken && | |
| !isLegacyToken && | |
| !isOAuthToken | |
| ) { | |
| throw new Error( | |
| "Invalid GitHub token format. Expected:\n" + | |
| "- Classic token: ghp_...\n" + | |
| "- Fine-grained token: github_pat_...\n" + | |
| "- GitHub App token: ghs_...\n" + | |
| "- OAuth token: 20–255 character string\n" + | |
| "- Legacy token: 40 character hexadecimal string" | |
| ); | |
| } |
🤖 Prompt for AI Agents
In src/lib/api/github-api-client.ts around lines 128 to 157, the fine-grained
token regex is too strict (enforces exact 82 chars) and the error text
incorrectly states 94 chars; change the validation to use a permissive
prefix/pattern for fine‑grained tokens (e.g. only verify it starts with
"github_pat_" and matches allowed characters) instead of enforcing a fixed
length, keep the other token prefix checks similarly permissive, and update the
thrown error text to remove the incorrect fixed-length claim (mention prefixes
and refer to the existing min/max length checks enforced later).
| if (overview.length === 0) { | ||
| console.warn('No repositories found, using demo overview'); | ||
| console.warn("No repositories found, using demo overview"); | ||
| return this.getUserAnalytics(username); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Infinite recursion when no repos found.
If overview is empty, calling getUserAnalytics() again loops indefinitely for users with no repos.
Return a sane fallback instead:
- if (overview.length === 0) {
- console.warn("No repositories found, using demo overview");
- return this.getUserAnalytics(username);
- }
+ if (overview.length === 0) {
+ console.warn("No repositories found; returning empty overview");
+ const behavior = await this.getWeeklyBehaviorData(username);
+ return { profile, overview: [], languages, behavior };
+ }Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/lib/api/github-api-client.ts around lines 892 to 895, the code currently
detects an empty overview and calls this.getUserAnalytics(username) again which
causes infinite recursion for users with no repos; replace the recursive call
with a sane fallback return (for example, return a default/empty analytics
object or a demo overview object consistent with the method's return type), log
a clear warning message including the username, and ensure the returned object
matches the expected shape so callers don't break (update or add a small unit
test to cover the no-repos case).
Summary by CodeRabbit
New Features
Localization
Chores