Website: new page for GitHub insights #926
Conversation
…tors and the number of their commits add key prop to each contributor component install dependencies for avatar remove profile_image from json; solve typescript error describe metadata accurately
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Visit the preview URL for this PR (updated for commit 1663200): https://si-admin-staging--pr926-sandino-open-source-nkhws17m.web.app (expires Tue, 17 Dec 2024 17:42:01 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: b7b0969384059dce6ea8fad1ee1d1737e54e6676 |
|
@activus-d new PR for you |
|
Issue: On the Vercel preview link for /open-source, the stats don't show up. Locally, everything works fine. Best guess: Since it’s running on a Vercel domain ( Screenshot from localhost: |
website/src/app/[lang]/[region]/(website)/open-source/(components)/contributors.tsx
Outdated
Show resolved
Hide resolved
ssandino
added
the
GitHub DPG
📝 WalkthroughWalkthroughThis pull request introduces several new components and functionalities for an open-source insights webpage, including localization strings in a new JSON file, various React components for displaying contributors and issues, and functions to fetch data from GitHub. The changes enhance the structure and presentation of open-source contributions, providing metrics and filtering capabilities for issues and contributors. Changes
Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
ssandino
changed the title
activus-d
requested review from
andrashee and
socialincome-dev
as code owners
![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
🧹 Outside diff range and nitpick comments (12)
website/src/app/[lang]/[region]/(website)/open-source/(components)/issue.tsx (2)
5-14: Consider adding JSDoc comments for better documentation.The interfaces are well-structured, but adding JSDoc comments would improve code documentation.
+/** + * Represents a GitHub issue with its core properties + */ interface Issue { id: number; title: string; url: string; labels: string[]; } +/** + * Props for the Issue component + */ interface IssueProps extends Pick<Issue, 'title' | 'url'> { text: string; }
16-29: Optimize CSS class usage.Consider extracting repeated classes and removing unnecessary hover styles.
export function Issue({ title, url, text }: IssueProps) { return ( - <TableRow className="p-5 hover:bg-transparent"> + <TableRow className="p-5"> - <TableCell className="text-xl">{title}</TableCell> - <TableCell className="text-right text-xl"> + <TableCell className="text-xl">{title}</TableCell> + <TableCell className="text-xl text-right"> <Button asChild variant="link" className="hover:underline"> <a href={url} target="_blank" rel="noopener noreferrer"> {text}website/src/app/[lang]/[region]/(website)/open-source/page.tsx (1)
8-17: Add type safety for section componentsThe implementation looks good. Consider adding Props interfaces for each section component to ensure type safety.
+ interface SectionProps { + lang: string; + region: string; + }shared/locales/en/website-open-source.json (1)
3-3: Remove trailing space in metadata titleThe title contains an unnecessary trailing space.
- "title": "Open-Source Insights ", + "title": "Open-Source Insights"website/src/app/[lang]/[region]/(website)/open-source/(sections)/hero.tsx (1)
15-19: Optimize title rendering performanceMove the title mapping outside the JSX to avoid unnecessary re-renders.
+ const titles = translator.t<{ text: string; color?: FontColor }[]>('hero.title-1'); return ( <div className="mx-auto mb-8 mt-20 flex w-4/5 flex-col items-center justify-center md:mb-10 lg:w-3/5"> <div className="mb-8 text-center"> - {translator.t<{ text: string; color?: FontColor }[]>('hero.title-1').map((title, index) => ( + {titles.map((title, index) => ( <Typography as="span" size="5xl" weight="medium" color={title.color} key={index}> {title.text}{' '} </Typography> ))}website/src/app/[lang]/[region]/(website)/open-source/(components)/filterForm.tsx (2)
31-38: Add aria-label to SelectContent for better accessibilityThe SelectContent component should have an aria-label for screen readers.
- <SelectContent> + <SelectContent aria-label="Available filter options">
33-37: Consider memoizing mapped labelsFor better performance with large label arrays, consider memoizing the mapped items.
+const memoizedLabels = useMemo( + () => labels.map((label) => ( + <SelectItem key={label} value={label}> + {label} + </SelectItem> + )), + [labels] +); // Then in JSX: - {labels.map((label) => ( - <SelectItem key={label} value={label}> - {label} - </SelectItem> - ))} + {memoizedLabels}website/src/app/[lang]/[region]/(website)/open-source/(components)/contributors-client.tsx (2)
42-42: Extract magic number to a named constantThe number 16 should be extracted to a named constant for better maintainability.
+const INITIAL_CONTRIBUTORS_LIMIT = 16; - const displayedContributors = showAllContributors ? contributors : contributors.slice(0, 16); + const displayedContributors = showAllContributors ? contributors : contributors.slice(0, INITIAL_CONTRIBUTORS_LIMIT);
67-73: Consider internationalizing button textThe "Show all" text should be internationalized for consistency with other UI elements.
website/src/app/[lang]/[region]/(website)/open-source/(components)/issues-client.tsx (2)
33-33: Extract magic number to a named constantThe number 6 should be extracted to a named constant for better maintainability.
+const INITIAL_ISSUES_LIMIT = 6; - const displayedIssues = showAllIssues ? filteredIssues : filteredIssues.slice(0, 6); + const displayedIssues = showAllIssues ? filteredIssues : filteredIssues.slice(0, INITIAL_ISSUES_LIMIT);
50-80: Add error boundary for robust error handlingConsider wrapping the component with an error boundary to gracefully handle runtime errors.
+class IssueClientErrorBoundary extends React.Component { + // Error boundary implementation +} +export function IssueClientWithErrorBoundary(props: IssueClientProps) { + return ( + <IssueClientErrorBoundary> + <IssueClient {...props} /> + </IssueClientErrorBoundary> + ); +}website/src/app/[lang]/[region]/(website)/open-source/(sections)/overview.tsx (1)
8-23: Simplify type definitions to reduce duplicationThe
OverviewIteminterface shares properties withOverviewProp. Consider consolidating these types:type OverviewProp = { title: string; total: number; time: string; }; -interface OverviewItem { - title: string; - time: string; -} +type OverviewItem = Omit<OverviewProp, 'total'>; interface Overview { forks: OverviewItem; commits: OverviewItem; stars: OverviewItem; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (8)
shared/locales/en/website-open-source.json(1 hunks)website/src/app/[lang]/[region]/(website)/open-source/(components)/contributors-client.tsx(1 hunks)website/src/app/[lang]/[region]/(website)/open-source/(components)/filterForm.tsx(1 hunks)website/src/app/[lang]/[region]/(website)/open-source/(components)/issue.tsx(1 hunks)website/src/app/[lang]/[region]/(website)/open-source/(components)/issues-client.tsx(1 hunks)website/src/app/[lang]/[region]/(website)/open-source/(sections)/hero.tsx(1 hunks)website/src/app/[lang]/[region]/(website)/open-source/(sections)/overview.tsx(1 hunks)website/src/app/[lang]/[region]/(website)/open-source/page.tsx(1 hunks)
🧰 Additional context used
📓 Learnings (4)
website/src/app/[lang]/[region]/(website)/open-source/(components)/issue.tsx (1)
Learnt from: activus-d
PR: socialincome-san/public#926
File: website/src/app/[lang]/[region]/(website)/open-source/(components)/issues-client.tsx:6-6
Timestamp: 2024-11-29T08:28:11.973Z
Learning: In `website/src/app/[lang]/[region]/(website)/open-source/(components)/issues-client.tsx`, the `Issue` interface redeclaration is acceptable even if it's already imported from './issue'.
website/src/app/[lang]/[region]/(website)/open-source/(components)/issues-client.tsx (1)
Learnt from: activus-d
PR: socialincome-san/public#926
File: website/src/app/[lang]/[region]/(website)/open-source/(components)/issues-client.tsx:6-6
Timestamp: 2024-11-29T08:28:11.973Z
Learning: In `website/src/app/[lang]/[region]/(website)/open-source/(components)/issues-client.tsx`, the `Issue` interface redeclaration is acceptable even if it's already imported from './issue'.
website/src/app/[lang]/[region]/(website)/open-source/(sections)/overview.tsx (1)
Learnt from: activus-d
PR: socialincome-san/public#926
File: website/src/app/[lang]/[region]/(website)/open-source/(sections)/overview.tsx:41-71
Timestamp: 2024-11-29T08:26:46.124Z
Learning: In the 'Overview' component within 'website/src/app/[lang]/[region]/(website)/open-source/(sections)/overview.tsx', adding loading states during data fetching from the GitHub API is not necessary.
website/src/app/[lang]/[region]/(website)/open-source/(components)/contributors-client.tsx (2)
Learnt from: activus-d
PR: socialincome-san/public#926
File: website/src/app/[lang]/[region]/(website)/open-source/(components)/contributors-client.tsx:6-10
Timestamp: 2024-11-29T08:28:12.233Z
Learning: In `contributors-client.tsx`, the `ContributorProp` type does not require the `id` field, even if related types in other components include it.
Learnt from: activus-d
PR: socialincome-san/public#926
File: website/src/app/[lang]/[region]/(website)/open-source/(sections)/contributors.tsx:10-25
Timestamp: 2024-11-29T06:09:31.569Z
Learning: In `contributors.tsx`, additional error handling and loading states in the `OpenSourceContributors` component are not required.
🪛 Biome (1.9.4)
website/src/app/[lang]/[region]/(website)/open-source/(components)/issues-client.tsx
[error] 8-8: Shouldn't redeclare 'Issue'. Consider to delete it or rename it.
'Issue' is defined here:
(lint/suspicious/noRedeclare)
🔇 Additional comments (2)
website/src/app/[lang]/[region]/(website)/open-source/(components)/issue.tsx (1)
1-3: LGTM! Client directive and imports are properly structured.
website/src/app/[lang]/[region]/(website)/open-source/(sections)/overview.tsx (1)
70-72: 🛠️ Refactor suggestion
Optimize data fetching and add error handling
The sequential API calls can be optimized, and error handling should be added.
-const { totalForks, newForks } = await getForkCount();
-const { totalStars, newStars } = await getStarCount();
-const { newCommits, totalCommits } = await getCommits();
+try {
+ const [
+ { totalForks, newForks },
+ { totalStars, newStars },
+ { newCommits, totalCommits }
+ ] = await Promise.all([
+ getForkCount(),
+ getStarCount(),
+ getCommits()
+ ]);
+} catch (error) {
+ console.error('Failed to fetch GitHub metrics:', error);
+ return (
+ <Typography className="text-destructive">
+ Failed to load GitHub metrics. Please try again later.
+ </Typography>
+ );
+}Likely invalid or redundant comment.
website/src/app/[lang]/[region]/(website)/open-source/(sections)/hero.tsx
Show resolved
Hide resolved
website/src/app/[lang]/[region]/(website)/open-source/(sections)/overview.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Nice! This looks good to me👍🏼
I made a small commit with a few cleanups, see :20ca4c8
If you wanted, you could combine some of the logic from the get-commits.ts, get-forks.ts, etc. files because they have quite a bit of overlapping functionality. Otherwise, it's all good😊
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Outside diff range and nitpick comments (13)
ui/package.json (1)
Line range hint
48-50: Revert to caret (^) version constraints for React packagesUsing
>=without an upper bound is risky as it could automatically pull in breaking changes from future major versions. Consider reverting to caret ranges:- "@types/react": ">=18.3.3", + "@types/react": "^18.3.3", - "react": ">=18.3.1", + "react": "^18.3.1", - "react-dom": ">=18.3.1", + "react-dom": "^18.3.1"website/src/app/[lang]/[region]/(website)/open-source/(components)/get-forks.ts (3)
11-17: Add retry mechanism for rate limits.While the code handles authentication well, consider implementing a retry mechanism with exponential backoff for rate limit errors.
+const wait = (ms: number) => new Promise(resolve => setTimeout(resolve, ms)); + +async function fetchWithRetry(url: string, options: RequestInit, maxRetries = 3): Promise<Response> { + for (let i = 0; i < maxRetries; i++) { + const response = await fetch(url, options); + if (response.status !== 403 || !response.headers.get('x-ratelimit-remaining')) { + return response; + } + await wait(Math.pow(2, i) * 1000); + } + throw new Error('Max retries reached'); +}
32-33: Add type safety for API response.The repository data is parsed without type checking. Consider adding an interface for the API response.
+interface GitHubRepoResponse { + forks_count: number; + [key: string]: unknown; +} + -const repoData = await repoRes.json(); +const repoData = (await repoRes.json()) as GitHubRepoResponse;
69-73: Optimize date comparison.The current implementation creates new Date objects in each iteration. Consider moving the comparison date outside the loop.
+const comparisonDate = new Date(startDateISO); for (const fork of forks) { - if (new Date(fork.created_at) >= new Date(startDateISO)) { + if (new Date(fork.created_at) >= comparisonDate) { newForks++; } }website/src/app/[lang]/[region]/(website)/open-source/(components)/get-stars.ts (3)
76-76: Optimize date parsing outside the loopCurrently,
new Date(startDateISO)is recreated on each iteration. Parsing it once outside the loop improves performance.Apply this diff to implement the change:
const startDateISO = startDate.toISOString(); +const startDateObj = new Date(startDateISO); // ... for (const star of stars) { const starredAt = new Date(star.starred_at); - if (starredAt >= new Date(startDateISO)) { + if (starredAt >= startDateObj) { newStars++; } }
14-16: Avoid unnecessary 'Accept' header in repository requestThe
Acceptheader'application/vnd.github.star+json'is only needed when fetching stargazers. Removing it from the repository info request simplifies the headers.Apply this diff to adjust the headers:
-const headers: Record<string, string> = { - Accept: 'application/vnd.github.star+json', -}; +const headers: Record<string, string> = {}; // Conditionally add the Authorization header if GITHUB_PAT is available if (process.env.GITHUB_PAT) { headers['Authorization'] = `Bearer ${process.env.GITHUB_PAT}`; } +const starHeaders = { ...headers, Accept: 'application/vnd.github.star+json' }; // Fetch repository info const repoRes = await fetch(repoUrl, { headers, }); // Fetch stargazers const pagedRes = await fetch(`${starUrl}&page=${page}`, { - headers, + headers: starHeaders, });
60-63: Clarify rate limit error messageWhen the rate limit is exceeded, users may need to verify that
GITHUB_PATis correctly set. Consider updating the error message to provide guidance.Apply this diff to update the error message:
throw new Error( - 'GitHub API rate limit exceeded during stargazers fetching. Please try again later or increase rate limit by authenticating.', + 'GitHub API rate limit exceeded during stargazers fetching. Please ensure that your GITHUB_PAT is correctly configured to increase the rate limit.' );website/src/app/[lang]/[region]/(website)/open-source/(components)/get-commits.ts (1)
37-49: Refactor duplicated error handling into a shared utilityThe error handling logic is duplicated in both API calls. Consider extracting this code into a helper function to adhere to the DRY principle and improve maintainability.
Also applies to: 59-69
website/src/app/[lang]/[region]/(website)/open-source/(components)/get-contributors.ts (1)
32-42: Refactor duplicated error handling into a shared utilityThe error handling code is similar across multiple functions. Consider creating a shared utility function to centralize error handling logic and promote code reuse.
website/src/app/[lang]/[region]/(website)/open-source/(components)/get-issues.ts (1)
35-47: Refactor duplicated error handling into a shared utilityThe error handling logic is duplicated across multiple functions. Consider extracting this code into a helper function to adhere to the DRY principle and improve maintainability.
website/src/app/[lang]/[region]/(website)/open-source/(components)/issues-client.tsx (3)
28-33: Make the display limit configurableThe hardcoded limit of 6 items should be made configurable via props for better reusability.
interface IssueClientProps { title: string; issues: Issue[]; labels: string[]; tableHeader: string; linkText: string; filterText: string; + displayLimit?: number; } -const displayedIssues = showAllIssues ? filteredIssues : filteredIssues.slice(0, 6); +const displayedIssues = showAllIssues ? filteredIssues : filteredIssues.slice(0, displayLimit ?? 6);
34-43: Separate concerns in handleFilterChangeConsider splitting the filter and show all functionality into separate handlers for better maintainability.
-const handleFilterChange = (label: string) => { +const handleShowAll = () => { + setFilteredLabel(''); + setShowAllIssues(true); +}; + +const handleFilterChange = (label: string) => { if (label === filterText) { - setFilteredLabel(''); - setShowAllIssues(true); + handleShowAll(); } else { setFilteredLabel(label); setShowAllIssues(false); } };
70-70: Remove hardcoded margin from buttonThe hardcoded
mr-20margin could cause layout issues. Consider using a more flexible layout approach.-className="text-card-foreground mr-20 text-xl" +className="text-card-foreground text-xl"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (9)
ui/package.json(1 hunks)website/src/app/[lang]/[region]/(website)/open-source/(components)/contributors-client.tsx(1 hunks)website/src/app/[lang]/[region]/(website)/open-source/(components)/get-commits.ts(1 hunks)website/src/app/[lang]/[region]/(website)/open-source/(components)/get-contributors.ts(1 hunks)website/src/app/[lang]/[region]/(website)/open-source/(components)/get-forks.ts(1 hunks)website/src/app/[lang]/[region]/(website)/open-source/(components)/get-issues.ts(1 hunks)website/src/app/[lang]/[region]/(website)/open-source/(components)/get-stars.ts(1 hunks)website/src/app/[lang]/[region]/(website)/open-source/(components)/issue.tsx(1 hunks)website/src/app/[lang]/[region]/(website)/open-source/(components)/issues-client.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- website/src/app/[lang]/[region]/(website)/open-source/(components)/issue.tsx
🧰 Additional context used
📓 Learnings (5)
website/src/app/[lang]/[region]/(website)/open-source/(components)/contributors-client.tsx (2)
Learnt from: activus-d
PR: socialincome-san/public#926
File: website/src/app/[lang]/[region]/(website)/open-source/(components)/contributors-client.tsx:6-10
Timestamp: 2024-11-29T08:28:12.233Z
Learning: In `contributors-client.tsx`, the `ContributorProp` type does not require the `id` field, even if related types in other components include it.
Learnt from: activus-d
PR: socialincome-san/public#926
File: website/src/app/[lang]/[region]/(website)/open-source/(sections)/contributors.tsx:10-25
Timestamp: 2024-11-29T06:09:31.569Z
Learning: In `contributors.tsx`, additional error handling and loading states in the `OpenSourceContributors` component are not required.
website/src/app/[lang]/[region]/(website)/open-source/(components)/get-stars.ts (1)
Learnt from: activus-d
PR: socialincome-san/public#926
File: website/src/app/[lang]/[region]/(website)/open-source/(components)/get-stars.tsx:51-85
Timestamp: 2024-11-29T07:52:05.401Z
Learning: In `website/src/app/[lang]/[region]/(website)/open-source/(components)/get-stars.tsx`, the function `getStarCount` does not require optimization to stop pagination when older stars are encountered, as this optimization is not relevant.
website/src/app/[lang]/[region]/(website)/open-source/(components)/get-contributors.ts (1)
Learnt from: activus-d
PR: socialincome-san/public#926
File: website/src/app/[lang]/[region]/(website)/open-source/(components)/get-contributors.tsx:28-30
Timestamp: 2024-11-29T08:25:44.729Z
Learning: In the React TypeScript file `get-contributors.tsx`, adding timeout and retry logic to the `getContributors` function's GitHub API calls is not necessary.
website/src/app/[lang]/[region]/(website)/open-source/(components)/get-issues.ts (2)
Learnt from: activus-d
PR: socialincome-san/public#926
File: website/src/app/[lang]/[region]/(website)/open-source/(components)/get-issues.tsx:55-72
Timestamp: 2024-11-29T08:27:15.145Z
Learning: In `website/src/app/[lang]/[region]/(website)/open-source/(components)/get-issues.tsx`, using `any` types in data processing is acceptable, and replacing them with interfaces is not necessary.
Learnt from: activus-d
PR: socialincome-san/public#926
File: website/src/app/[lang]/[region]/(website)/open-source/(components)/get-issues.tsx:30-33
Timestamp: 2024-11-29T08:27:39.910Z
Learning: In the `getIssuesData` function located at `website/src/app/[lang]/[region]/(website)/open-source/(components)/get-issues.tsx`, adding request timeout and retry logic is not necessary.
website/src/app/[lang]/[region]/(website)/open-source/(components)/issues-client.tsx (1)
Learnt from: activus-d
PR: socialincome-san/public#926
File: website/src/app/[lang]/[region]/(website)/open-source/(components)/issues-client.tsx:6-6
Timestamp: 2024-11-29T08:28:11.973Z
Learning: In `website/src/app/[lang]/[region]/(website)/open-source/(components)/issues-client.tsx`, the `Issue` interface redeclaration is acceptable even if it's already imported from './issue'.
🪛 Biome (1.9.4)
website/src/app/[lang]/[region]/(website)/open-source/(components)/issues-client.tsx
[error] 8-8: Shouldn't redeclare 'Issue'. Consider to delete it or rename it.
'Issue' is defined here:
(lint/suspicious/noRedeclare)
🔇 Additional comments (7)
ui/package.json (1)
27-27: LGTM: Avatar dependency addition
The addition of @radix-ui/react-avatar is appropriate for the new Avatar components being implemented.
website/src/app/[lang]/[region]/(website)/open-source/(components)/get-stars.ts (1)
1-87: Implementation is correct and follows best practices
The getStarCount function effectively retrieves total stars and counts new stars within the last 30 days, with proper error handling and pagination.
website/src/app/[lang]/[region]/(website)/open-source/(components)/get-contributors.ts (1)
20-63: Implementation of getContributors function is correct
The getContributors function effectively retrieves and processes contributor data, mapping it appropriately.
website/src/app/[lang]/[region]/(website)/open-source/(components)/get-issues.ts (1)
16-82: getIssuesData function correctly fetches and processes issues
The function successfully retrieves open issues, handles pagination, and maps the data appropriately.
website/src/app/[lang]/[region]/(website)/open-source/(components)/contributors-client.tsx (2)
12-29: Contributor component is well-implemented
The Contributor component correctly displays contributor information with appropriate styling.
31-76: OpenSourceContributorsClient component functions effectively
The component properly manages state to display contributors and handles the "Show all" functionality appropriately.
website/src/app/[lang]/[region]/(website)/open-source/(components)/issues-client.tsx (1)
24-27: Verify prop types usage in the codebase
The component's props are well-structured, but let's verify their usage across the codebase.
✅ Verification successful
Props usage appears correct and consistent
Based on the ripgrep results, the IssueClient component is properly imported and used in open-issues.tsx with the expected props structure. The component's props interface matches its usage pattern.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for IssueClient component usage
ast-grep --pattern 'IssueClient $$$'
Length of output: 38
Script:
#!/bin/bash
# Search for IssueClient component usage with a more specific pattern
ast-grep --pattern '<IssueClient $$$'
# Also search with ripgrep for any usage
rg "IssueClient" -A 2
# Search for the file where IssueClient is imported
rg "import.*IssueClient"
Length of output: 1654
Script:
#!/bin/bash
# Let's check the actual usage context in open-issues.tsx
cat website/src/app/[lang]/[region]/(website)/open-source/(sections)/open-issues.tsx
Length of output: 267
| const errorDetails = await repoRes.text(); | ||
| const status = repoRes.status; | ||
| if (status === 403) { | ||
| throw new Error(`GitHub API rate limit exceeded: ${status} - ${errorDetails}.`); |
There was a problem hiding this comment.
Avoid exposing sensitive details in error messages.
Error messages might expose rate limit details that could be used for timing attacks. Consider using generic messages in production.
-throw new Error(`GitHub API rate limit exceeded: ${status} - ${errorDetails}.`);
+throw new Error('Unable to fetch repository data. Please try again later.');
-throw new Error(
- 'GitHub API rate limit exceeded during forks fetching. Please try again later or increase rate limit by authenticating.',
-);
+throw new Error('Unable to fetch fork data. Please try again later.');Also applies to: 57-57
| const owner = 'socialincome-san'; | ||
| const repo = 'public'; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider parameterizing repository details.
The hardcoded repository details limit the function's reusability. Consider accepting these as parameters.
-const owner = 'socialincome-san';
-const repo = 'public';
+export const DEFAULT_OWNER = 'socialincome-san';
+export const DEFAULT_REPO = 'public';
+
+export async function getForkCount(
+ owner: string = DEFAULT_OWNER,
+ repo: string = DEFAULT_REPO
+): Promise<{ totalForks: number; newForks: number }> {Committable suggestion skipped: line range outside the PR's diff.
| if (process.env.GITHUB_PAT) { | ||
| headers['Authorization'] = `Bearer ${process.env.GITHUB_PAT}`; | ||
| } |
There was a problem hiding this comment.
Ensure GITHUB_PAT is not exposed to client-side code
Accessing process.env.GITHUB_PAT should be restricted to server-side code to prevent exposing sensitive information. Confirm that this function runs only on the server side.
| let totalCommits = 1; | ||
|
|
||
| if (linkHeader) { | ||
| const match = linkHeader.match(/&page=(\d+)>; rel="last"/); | ||
| if (match) { | ||
| totalCommits = parseInt(match[1], 10); | ||
| } else { | ||
| console.warn('Link header exists but could not parse total commits page count.'); | ||
| } | ||
| } else { | ||
| console.warn('No Link header found; assuming a single commit.'); | ||
| } |
There was a problem hiding this comment.
Handle repositories with zero commits correctly
When the Link header is absent, totalCommits defaults to 1. This may be inaccurate for repositories with zero commits. Consider adjusting the logic to account for this scenario to ensure accurate commit counts.
| if (process.env.GITHUB_PAT) { | ||
| headers['Authorization'] = `Bearer ${process.env.GITHUB_PAT}`; | ||
| } |
There was a problem hiding this comment.
Ensure GITHUB_PAT is not exposed to client-side code
Accessing process.env.GITHUB_PAT should be restricted to server-side code to prevent exposing secrets. Verify that this function runs only on the server side.
| headers['Authorization'] = `Bearer ${process.env.GITHUB_PAT}`; | ||
| } |
There was a problem hiding this comment.
Ensure GITHUB_PAT is not exposed to client-side code
Accessing process.env.GITHUB_PAT should be restricted to server-side code to prevent exposing sensitive information. Confirm that this function runs only on the server side.
Thank you for the feedback, @mkue. I'll add the logic combination to my tasks. For learning purposes, I'm curious about why you changed the file format of some fetching files from |
The |
This PR solves issue #881 by introducing the following changes:
Phase 1:
- Implements the layout and structure for the new /open-source page.
- Adds a new English (en) translation file containing text elements specific to the /open-source page.
Phase 2:
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation
Chores