refactor: created npm folder and modularized scripts #4192
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sagarkori143
requested review from
Mayaleeeee,
akshatnema,
anshgoyalevil,
asyncapi-bot-eve,
derberg and
sambhavgupta0705
as code owners
✅ Deploy Preview for asyncapi-website ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis update introduces a suite of new runner scripts in the Changes
Sequence Diagram(s)sequenceDiagram
participant RunnerScript
participant CoreScript
participant Logger
participant CustomError
RunnerScript->>CoreScript: Call build function (e.g., buildDashboard)
CoreScript-->>RunnerScript: Success or throws error
alt Error occurs
RunnerScript->>CustomError: Wrap error with context
RunnerScript->>Logger: Log error with context
RunnerScript-->>Caller: Rethrow CustomError or exit process
else Success
RunnerScript-->>Caller: Return/complete
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## add-integration-tests #4192 +/- ##
=======================================================
Coverage 100.00% 100.00%
=======================================================
Files 22 21 -1
Lines 778 828 +50
Branches 144 165 +21
=======================================================
+ Hits 778 828 +50 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 31
♻️ Duplicate comments (2)
npm/scripts/helpers/utils.ts (2)
14-33: Second copy of convertToJson ‒ unify with utils.tsSee comments in
npm/scripts/utils.ts. Having two versions invites drift (e.g., one handles edge case, the other doesn’t). Centralise.
40-44: pause duplicationSame argument as above – keep one implementation, export from a shared place.
🧹 Nitpick comments (35)
npm/scripts/helpers/logger.ts (1)
5-16: Minor: prefer adefaultexport for ergonomicsNearly every caller will want the singleton logger:
-export { logger }; +export default logger;This avoids
{ logger }noise at call-sites while still allowing named import (import logger from …) when TSesModuleInteropis enabled.
If you keep the named export, ensure it is re-exported from an index barrel to minimise relative-path churn.npm/scripts/utils/logger.ts (1)
6-15: Consider environment-specific transportsA single console transport is fine for local dev, but in CI you may want:
- JSON format for CloudWatch / GCP logs
level: 'warn'by default on productionExample:
- transports: [new winston.transports.Console()] + transports: [ + new winston.transports.Console({ + level: process.env.CI ? 'warn' : (process.env.LOG_LEVEL || 'info') + }) + ]npm/scripts/utils.ts (1)
41-45: pause helper: support AbortSignal to avoid dangling timersIt’s handy for long-running CLI tasks to cancel waits on SIGINT.
-async function pause(ms: number): Promise<void> { - return new Promise((res) => { - setTimeout(res, ms); - }); +async function pause(ms: number, signal?: AbortSignal): Promise<void> { + return new Promise((res, rej) => { + const id = setTimeout(res, ms); + signal?.addEventListener('abort', () => { + clearTimeout(id); + rej(new Error('Paused operation aborted')); + }); + }); }npm/scripts/tools/categorylist.ts (1)
3-106: Consider externalising category data & assert uniquenessThe large literal array works, but a JSON/YAML file under
data/would:
- Let non-dev contributors edit categories without touching TS.
- Allow runtime validation in CI (e.g., no duplicate
tag).Even if you keep it inline, add a quick uniqueness guard:
const tags = new Set(categoryList.map(c => c.tag)); if (tags.size !== categoryList.length) { throw new Error('Duplicate tool category tags detected'); }This will prevent accidental collisions like
'code-generator'vs'generator'.npm/scripts/tools/tags-color.ts (2)
4-85: Consider making the color table immutable and eliminating repeated literalsThe array is purely data. Declaring it with
as const(orReadonlyArray) prevents accidental mutation and gives the compiler literal-type precision.
Second, every object repeats the same keys (name,color,borderColor). A helper likemakeTag('Go/Golang', '#8ECFDF', '#00AFD9')would DRY the table and make edits less error-prone.-const languagesColor: LanguageColorItem[] = [ +export const languagesColor = [ /* … */ -] ; +] as const satisfies ReadonlyArray<LanguageColorItem>;Optional factory:
const tag = (name: string, bg: string, border: string): LanguageColorItem => ({ name, color: `bg-[${bg}]`, borderColor: `border-[${border}]`, });
87-173: Typos / inconsistencies in tag names
Saas→SaaS,Nest Js→NestJS,Springboot→Spring Boot.
Minor, but these names may be surfaced in the UI and become search keys, so correctness matters.package.json (1)
29-31:build:allduplicates existing pipeline
"build"already chains"build-scripts"which triggers pages + posts.
build:allruns the new orchestrator but does not include the old steps (lint, pages build, etc.), and nothing in CI calls it yet.
Decide which command is canonical and reference it everywhere (CI, Makefile, Netlify). Otherwise different environments will build different assets.npm/scripts/utils/readAndWriteJson.ts (1)
30-34: Pretty-print the written JSON for VCS diff-friendliness
JSON.stringify(jsonContent)writes a compact single-line file. Adding indentation makes diffs readable and keeps tooling output consistent.- await writeFile(writePath, JSON.stringify(jsonContent)); + await writeFile(writePath, JSON.stringify(jsonContent, null, 2));npm/runners/build-dashboard-runner.ts (1)
10-13: Hard-coded output path can break when runner is moved
resolve(currentDirPath, '..', '..', 'dashboard.json')assumes the runner stays two levels below repo root. Consider passing the destination path from the orchestrator or usingprocess.cwd()to keep relative to project root.const writePath = resolve(process.cwd(), 'dashboard.json');npm/scripts/casestudies/index.ts (2)
27-27: Pretty-print the generated JSON for Git reviews-await writeFile(writeFilePath, JSON.stringify(caseStudiesList)); +await writeFile(writeFilePath, JSON.stringify(caseStudiesList, null, 2));
31-31: Preserve original stack trace withcauseinstead of a second parameter-throw new Error(err instanceof Error ? err.message : String(err)); +throw new Error('Failed to build case-studies list', { cause: err });npm/runners/build-tools-runner.ts (2)
17-20: Redundant.catch& hardprocess.exit(1)inside library codeLet the outer
try/catchpropagate the error; callers decide how to terminate.-await buildTools(...).catch((err) => { - logger.error('Failed to build tools:', err); - process.exit(1); -}); +await buildTools(automatedToolsPath, manualToolsPath, toolsPath, tagsPath);
22-22: SameErrorconstructor misuse as other runners-throw new Error('Error building tools: ', error as Error); +throw new Error(`Error building tools: ${(error as Error).message}`, { cause: error });npm/runners/build-post-list-runner.ts (1)
21-23: Use correctErrorconstruction pattern-throw new Error('Error building post list: ', err as Error); +throw new Error(`Error building post list: ${(err as Error).message}`, { cause: err });npm/scripts/tools/extract-tools-github.ts (1)
28-30: Parameter should beperPage, notPerPageCapitalised variable names read like types.
- const getReqUrl = (PerPage: number, pageNo: number) => - `https://api.github.com/search/code?q=filename:.asyncapi-tool&per_page=${PerPage}&page=${pageNo}`; + const getReqUrl = (perPage: number, pageNo: number) => + `https://api.github.com/search/code?q=filename:.asyncapi-tool&per_page=${perPage}&page=${pageNo}`;npm/scripts/build-tools.ts (2)
1-9: Remove unusedloggerimport
loggerisn’t referenced and will tripnoUnusedLocals.-import { logger } from './utils/logger';
22-35: Retain original stack viacauseWrapping the error without
causeloses the stack trace.- } catch (err) { - throw new Error(`An error occurred while building tools: ${(err as Error).message}`); + } catch (err) { + throw new Error('An error occurred while building tools', { cause: err as Error }); }npm/index.ts (1)
12-46: Five identicaltry/catchblocks — compress into a loopRefactor to an array of labelled tasks to reduce noise and future maintenance overhead.
const tasks: [string, () => Promise<unknown>][] = [ ['posts', runBuildPostList], ['dashboard', runBuildDashboard], ['tools', runBuildTools], ['case studies', runCaseStudies], ['newsroom videos', runBuildNewsroomVideos], ]; for (const [label, task] of tasks) { try { await task(); } catch (err) { errorFaced = true; logger.error(`Error building ${label}:`, err as Error); } }npm/scripts/build-newsroom-videos.ts (2)
4-12: Prune unused imports/variables
resolve,currentFilePath, andcurrentDirPathare never used.-import { dirname, resolve } from 'path'; +import { dirname } from 'path'; … -const currentFilePath = fileURLToPath(import.meta.url); -const currentDirPath = dirname(currentFilePath);
69-71: Preserve original error- } catch (err) { - throw new Error(`Failed to build newsroom videos: ${(err as Error).message}`); + } catch (err) { + throw new Error('Failed to build newsroom videos', { cause: err as Error }); }npm/scripts/index.ts (1)
11-13: Dead code:currentFilePath¤tDirPathThey’re never used and break
noUnusedLocals.-const currentFilePath = fileURLToPath(import.meta.url); -const currentDirPath = dirname(currentFilePath);npm/scripts/build-meetings.ts (3)
36-36: Simplify date instantiation.
Date.now()is redundant when creating a new Date object.- const currentTime = new Date(Date.now()).toISOString(); + const currentTime = new Date().toISOString();
59-62: Use optional chaining for cleaner code.The conditional logic can be simplified using optional chaining as suggested by static analysis.
- url: - e.extendedProperties?.private && - `https://github.com/asyncapi/community/issues/${e.extendedProperties.private.ISSUE_ID}`, - banner: e.extendedProperties?.private && e.extendedProperties.private.BANNER, + url: e.extendedProperties?.private?.ISSUE_ID + ? `https://github.com/asyncapi/community/issues/${e.extendedProperties.private.ISSUE_ID}` + : undefined, + banner: e.extendedProperties?.private?.BANNER,
71-71: Use async file operations for consistency.Since this is an async function, consider using the async version of file write operations.
Import the async version at the top:
-import { writeFileSync } from 'fs'; +import { writeFile } from 'fs/promises';Then update the write operation:
- writeFileSync(writePath, eventsForHuman); + await writeFile(writePath, eventsForHuman);npm/scripts/casestudies/casestudy_template.yml (1)
22-22: Fix typo in comment.There's a typo in the word "languages".
- - #Provide a list of programming lanugages people using AsyncAPI work with. + - #Provide a list of programming languages people using AsyncAPI work with.npm/scripts/build-docs.ts (1)
234-234: Consider simplifying complex ternary expression.The ternary expression for constructing the prevPage title is difficult to read and maintain.
- title: `${structuredPosts[index - 1]?.isRootSection ? rootSections[rootSections.length - 2] : rootSections[rootSections.length - 1]} - ${structuredPosts[index - 2].title}`, + const sectionIndex = structuredPosts[index - 1]?.isRootSection + ? rootSections.length - 2 + : rootSections.length - 1; + prevPage = { + title: `${rootSections[sectionIndex]} - ${structuredPosts[index - 2].title}`, + href: structuredPosts[index - 2].slug + };npm/scripts/build-post-list.ts (1)
1-2: Consider refactoring to avoid disabling ESLint rules.The ESLint rules are disabled due to complex async logic in loops. Consider refactoring the
walkDirectoriesfunction to process directories in parallel where possible or extract the loop body into a separate function.You could use
Promise.all()for parallel processing of independent operations or extract the complex loop logic into smaller, more focused functions to improve readability and avoid the need for ESLint rule suppression.npm/scripts/tools/tools-schema.json (2)
45-76: Over-engineeredlanguagedefinition
language.anyOf → string.anyOf → enumis three-levels deep yet always ends in the same enum / free-text.
You can flatten it to a singleanyOfarray and drop one nested schema object to cut ~30 lines without changing validation behaviour.
118-136: Inconsistent tag spellingValues such as
"Node js"/"React JS"mix spaces & casing while the rest of the ecosystem uses"Node.js"/"ReactJS".
String mismatches will break the Fuse look-ups later on because they rely on literal equality.npm/scripts/dashboard/issue-queries.ts (1)
210-215:type: ISSUEwithis:pull-requestis redundantWhen you already filter with
is:pull-request, settingtype: ISSUEis unnecessary and slightly slower for the search API.npm/scripts/tools/tools-object.ts (1)
40-47: Parameter type drift
isAsyncAPIrepois typedboolean | string = ''but all call-sites pass a boolean.
Keep it strictly boolean to avoid accidental string truthiness.- isAsyncAPIrepo: boolean | string = '' + isAsyncAPIrepo: boolean = falsenpm/scripts/tools/combine-tools.ts (1)
46-48: Fuse indexes rebuilt but not reusedEvery time an unknown tag is added the code re-creates the Fuse instance, which is O(n).
For large tag sets preferfuse.add()or maintain a Map for O(1) look-ups.npm/scripts/casestudies/schema.json (1)
17-20:idpattern is overly strict
"^[a-z]+$"forbids digits and hyphens (adeo-group,case123) which are common slug patterns.
Consider^[a-z0-9-]+$unless there is a strong requirement.npm/scripts/dashboard/build-dashboard.ts (2)
1-5: Remove unused imports/locals to satisfynoUnusedLocals.
resolve,currentFilePath, andcurrentDirPathare never referenced in active code (only in commented-out lines). Retain them only if you plan to re-enable the CLI entry-point; otherwise drop them or wrap the legacy code in a separate script to keep the build clean.Also applies to: 21-23
316-317: Prefer centralised logger overconsole.log.Stick to
logger.info/debugto keep log formatting consistent across scripts.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (31)
npm/index.ts(1 hunks)npm/runners/build-dashboard-runner.ts(1 hunks)npm/runners/build-newsroom-videos-runner.ts(1 hunks)npm/runners/build-post-list-runner.ts(1 hunks)npm/runners/build-tools-runner.ts(1 hunks)npm/runners/case-studies-runner.ts(1 hunks)npm/scripts/build-docs.ts(1 hunks)npm/scripts/build-meetings.ts(1 hunks)npm/scripts/build-newsroom-videos.ts(1 hunks)npm/scripts/build-post-list.ts(1 hunks)npm/scripts/build-tools.ts(1 hunks)npm/scripts/casestudies/casestudy_template.yml(1 hunks)npm/scripts/casestudies/index.ts(1 hunks)npm/scripts/casestudies/schema.json(1 hunks)npm/scripts/dashboard/build-dashboard.ts(1 hunks)npm/scripts/dashboard/issue-queries.ts(1 hunks)npm/scripts/helpers/check-locales.ts(1 hunks)npm/scripts/helpers/logger.ts(1 hunks)npm/scripts/helpers/readAndWriteJson.ts(1 hunks)npm/scripts/helpers/utils.ts(1 hunks)npm/scripts/index.ts(1 hunks)npm/scripts/tools/categorylist.ts(1 hunks)npm/scripts/tools/combine-tools.ts(1 hunks)npm/scripts/tools/extract-tools-github.ts(1 hunks)npm/scripts/tools/tags-color.ts(1 hunks)npm/scripts/tools/tools-object.ts(1 hunks)npm/scripts/tools/tools-schema.json(1 hunks)npm/scripts/utils.ts(1 hunks)npm/scripts/utils/logger.ts(1 hunks)npm/scripts/utils/readAndWriteJson.ts(1 hunks)package.json(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (9)
npm/runners/build-dashboard-runner.ts (1)
npm/scripts/dashboard/build-dashboard.ts (1)
start(305-323)
npm/scripts/helpers/readAndWriteJson.ts (3)
npm/scripts/utils/readAndWriteJson.ts (1)
writeJSON(12-36)npm/scripts/helpers/utils.ts (1)
convertToJson(46-46)npm/scripts/utils.ts (1)
convertToJson(47-47)
npm/scripts/casestudies/index.ts (2)
npm/scripts/helpers/utils.ts (1)
convertToJson(46-46)npm/scripts/utils.ts (1)
convertToJson(47-47)
npm/runners/build-post-list-runner.ts (1)
npm/scripts/build-post-list.ts (1)
buildPostList(288-314)
npm/scripts/utils/readAndWriteJson.ts (1)
npm/scripts/helpers/readAndWriteJson.ts (1)
writeJSON(14-40)
npm/scripts/tools/categorylist.ts (1)
types/scripts/tools.ts (1)
CategoryListItem(28-32)
npm/runners/case-studies-runner.ts (1)
npm/scripts/casestudies/index.ts (1)
buildCaseStudiesList(12-33)
npm/index.ts (7)
npm/runners/build-post-list-runner.ts (1)
runBuildPostList(10-26)npm/scripts/helpers/logger.ts (1)
logger(18-18)npm/scripts/utils/logger.ts (1)
logger(18-18)npm/runners/build-dashboard-runner.ts (1)
runBuildDashboard(10-17)npm/runners/build-tools-runner.ts (1)
runBuildTools(10-24)npm/runners/case-studies-runner.ts (1)
runCaseStudies(10-24)npm/runners/build-newsroom-videos-runner.ts (1)
runBuildNewsroomVideos(10-17)
npm/scripts/tools/tags-color.ts (1)
types/scripts/tools.ts (1)
LanguageColorItem(34-38)
🪛 Biome (1.9.4)
npm/scripts/tools/extract-tools-github.ts
[error] 60-60: The catch clause that only rethrows the original error is useless.
An unnecessary catch clause can be confusing.
Unsafe fix: Remove the try/catch clause.
(lint/complexity/noUselessCatch)
npm/index.ts
[error] 9-9: Don't use 'Boolean' as a type.
Use lowercase primitives for consistency.
Safe fix: Use 'boolean' instead
(lint/complexity/noBannedTypes)
npm/scripts/build-meetings.ts
[error] 62-62: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🪛 YAMLlint (1.37.1)
npm/scripts/casestudies/casestudy_template.yml
[warning] 22-22: wrong indentation: expected 2 but found 4
(indentation)
[warning] 24-24: wrong indentation: expected 2 but found 4
(indentation)
[warning] 26-26: wrong indentation: expected 2 but found 4
(indentation)
[warning] 45-45: wrong indentation: expected 2 but found 4
(indentation)
[error] 50-50: trailing spaces
(trailing-spaces)
[warning] 64-64: too many spaces after colon
(colons)
[error] 64-64: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 180000ms (2)
- GitHub Check: Lighthouse CI
- GitHub Check: Test NodeJS PR - macos-13
🔇 Additional comments (6)
npm/scripts/tools/tags-color.ts (1)
90-126: Tailwind “arbitrary value” classes require safelistingClasses like
bg-[#BDFF67]orborder-[#2596ED]are only generated in JIT mode when the exact string appears in source. If the JSON produced by other scripts inserts these classes dynamically, the values will be purged at build time unless they are safelisted intailwind.config.js.
Please confirm that the safelist already includes the full set, or generate conventional palette classes instead.npm/runners/case-studies-runner.ts (1)
6-8:projectRootresolves tonpm/not repository root
../../only takes you fromnpm/runners→npm.
Every subsequent path (config,pages, …) will therefore be wrong.-const projectRoot = resolve(currentDirPath, '../../'); +const projectRoot = resolve(currentDirPath, '../../..'); // runners → npm → repo-rootLikely an incorrect or invalid review comment.
npm/runners/build-post-list-runner.ts (1)
6-8:projectRootoff by one directory-const projectRoot = resolve(currentDirPath, '../../'); +const projectRoot = resolve(currentDirPath, '../../..');Likely an incorrect or invalid review comment.
npm/scripts/index.ts (1)
28-33: A single failure aborts the rest of the pipelineConsider guarding each awaited call in its own
try/catch, mirroring the pattern innpm/index.ts, so that one failing builder doesn’t stop the others.npm/scripts/build-docs.ts (1)
149-149: Fix error instantiation syntax.The error should be instantiated with the
newkeyword.- throw new Error('Error in convertDocPosts:', err as Error); + throw new Error(`Error in convertDocPosts: ${(err as Error).message}`);Likely an incorrect or invalid review comment.
npm/scripts/helpers/check-locales.ts (1)
1-162: Well-structured locale validation module!The implementation provides comprehensive validation for locale files with good error handling, clear logging, and efficient key comparison logic. The recursive key extraction and cross-language validation approach is solid.
npm/scripts/utils.ts
Outdated
Comment on lines
11
to
34
| function convertToJson(contentYAMLorJSON: unknown): any { | ||
| // Axios handles conversion to JSON by default, if data returned from the server allows it | ||
| // So if returned content is not a string (not YAML), we just return JSON back | ||
| if (typeof contentYAMLorJSON !== 'string') { | ||
| return contentYAMLorJSON; | ||
| } | ||
|
|
||
| // Check if the content is valid JSON before attempting to parse as YAML | ||
| try { | ||
| const jsonContent = JSON.parse(contentYAMLorJSON); | ||
|
|
||
| return jsonContent; | ||
| } catch (jsonError) { | ||
| // If it's not valid JSON, try parsing it as YAML | ||
| try { | ||
| const yamlContent = yaml.parse(contentYAMLorJSON); | ||
|
|
||
| return yamlContent; | ||
| } catch (yamlError) { | ||
| // If parsing as YAML also fails, throw an error | ||
| throw new Error(`Invalid content format:\nJSON Parse Error: ${jsonError}\nYAML Parse Error: ${yamlError}`); | ||
| } | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion
convertToJson duplicated & untyped — extract once and generify
The exact function is repeated in helpers/utils.ts. Extract to a single module (@/scripts/utils/convertToJson.ts) and reuse.
Additional polish:
- Make it generic to retain type info.
-function convertToJson(contentYAMLorJSON: unknown): any {
+function convertToJson<T = unknown>(contentYAMLorJSON: string | T): T {- When composing the thrown error, stringify the inner errors – the current template will emit
[object Object].
📝 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.
Suggested change
| function convertToJson(contentYAMLorJSON: unknown): any { | |
| // Axios handles conversion to JSON by default, if data returned from the server allows it | |
| // So if returned content is not a string (not YAML), we just return JSON back | |
| if (typeof contentYAMLorJSON !== 'string') { | |
| return contentYAMLorJSON; | |
| } | |
| // Check if the content is valid JSON before attempting to parse as YAML | |
| try { | |
| const jsonContent = JSON.parse(contentYAMLorJSON); | |
| return jsonContent; | |
| } catch (jsonError) { | |
| // If it's not valid JSON, try parsing it as YAML | |
| try { | |
| const yamlContent = yaml.parse(contentYAMLorJSON); | |
| return yamlContent; | |
| } catch (yamlError) { | |
| // If parsing as YAML also fails, throw an error | |
| throw new Error(`Invalid content format:\nJSON Parse Error: ${jsonError}\nYAML Parse Error: ${yamlError}`); | |
| } | |
| } | |
| } | |
| function convertToJson<T = unknown>(contentYAMLorJSON: string | T): T { | |
| // Axios handles conversion to JSON by default, if data returned from the server allows it | |
| // So if returned content is not a string (not YAML), we just return JSON back | |
| if (typeof contentYAMLorJSON !== 'string') { | |
| return contentYAMLorJSON; | |
| } | |
| // Check if the content is valid JSON before attempting to parse as YAML | |
| try { | |
| const jsonContent = JSON.parse(contentYAMLorJSON); | |
| return jsonContent; | |
| } catch (jsonError) { | |
| // If it's not valid JSON, try parsing it as YAML | |
| try { | |
| const yamlContent = yaml.parse(contentYAMLorJSON); | |
| return yamlContent; | |
| } catch (yamlError) { | |
| // If parsing as YAML also fails, throw an error | |
| throw new Error(`Invalid content format:\nJSON Parse Error: ${jsonError}\nYAML Parse Error: ${yamlError}`); | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In npm/scripts/utils.ts around lines 11 to 34, the convertToJson function is
duplicated in helpers/utils.ts and lacks type safety. Extract this function into
a new single module at @/scripts/utils/convertToJson.ts and import it where
needed to avoid duplication. Refactor the function to be generic so it preserves
and returns the inferred type of the parsed content. Also, update the error
handling to stringify the inner JSON and YAML parse errors when composing the
thrown error message to avoid outputting [object Object].
Comment on lines
1
to
40
| import { readFile, writeFile } from 'fs/promises'; | ||
|
|
||
| import { convertToJson } from './utils'; | ||
|
|
||
| /** | ||
| * Reads the content of a file, converts it to JSON, and writes the JSON output to another file. | ||
| * | ||
| * This asynchronous function processes a file by reading its content, converting the content to JSON using a conversion utility, and writing the resulting JSON to the specified destination path. Each operation is wrapped in error handling, with an error thrown if any step fails. | ||
| * | ||
| * @param readPath - The file path from which to read the content. | ||
| * @param writePath - The file path where the JSON output will be written. | ||
| * @throws {Error} If reading the file, converting its content to JSON, or writing the JSON output fails. | ||
| */ | ||
| export async function writeJSON(readPath: string, writePath: string) { | ||
| let readContent; | ||
| let jsonContent; | ||
|
|
||
| // Attempt to read the file | ||
| try { | ||
| readContent = await readFile(readPath, 'utf-8'); | ||
| } catch (err) { | ||
| return Promise.reject(err); | ||
| } | ||
|
|
||
| // Attempt to convert content to JSON | ||
| try { | ||
| jsonContent = convertToJson(readContent); | ||
| } catch (err) { | ||
| return Promise.reject(err); | ||
| } | ||
|
|
||
| // Attempt to write the JSON content to file | ||
| try { | ||
| await writeFile(writePath, JSON.stringify(jsonContent)); | ||
| } catch (err) { | ||
| return Promise.reject(err); | ||
| } | ||
|
|
||
| return Promise.resolve(); | ||
| } |
Contributor
There was a problem hiding this comment.
Duplicate implementation detected
writeJSON is defined here and in npm/scripts/utils/readAndWriteJson.ts. Divergent logic (throw vs reject) risks subtle bugs.
Action items:
- Pick one canonical location (e.g.,
scripts/utils/file.ts) and re-export from elsewhere. - Remove the duplicate to avoid bit-rot.
🤖 Prompt for AI Agents
In npm/scripts/helpers/readAndWriteJson.ts lines 1 to 40, the writeJSON function
duplicates the implementation found in npm/scripts/utils/readAndWriteJson.ts,
causing potential inconsistencies. To fix this, remove the writeJSON function
from this helpers file and instead re-export the canonical implementation from
the utils file here if needed. This eliminates duplication and ensures a single
source of truth for the function.
Comment on lines
18
to
37
| // Attempt to read the file | ||
| try { | ||
| readContent = await readFile(readPath, 'utf-8'); | ||
| } catch (err) { | ||
| return Promise.reject(err); | ||
| } | ||
|
|
||
| // Attempt to convert content to JSON | ||
| try { | ||
| jsonContent = convertToJson(readContent); | ||
| } catch (err) { | ||
| return Promise.reject(err); | ||
| } | ||
|
|
||
| // Attempt to write the JSON content to file | ||
| try { | ||
| await writeFile(writePath, JSON.stringify(jsonContent)); | ||
| } catch (err) { | ||
| return Promise.reject(err); | ||
| } |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion
Return/throw semantics are inconsistent with other helper
Inside an async function, throw already yields a rejected promise, so returning Promise.reject() is redundant and obscures stack traces.
The sibling file npm/scripts/utils/readAndWriteJson.ts throws errors instead—two subtly different implementations of the same helper now exist.
- } catch (err) {
- return Promise.reject(err);
+ } catch (err) {
+ throw err;
}Recommend:
- Keep a single implementation (see duplication note below).
- Use
throw, not manualPromise.reject.
📝 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.
Suggested change
| // Attempt to read the file | |
| try { | |
| readContent = await readFile(readPath, 'utf-8'); | |
| } catch (err) { | |
| return Promise.reject(err); | |
| } | |
| // Attempt to convert content to JSON | |
| try { | |
| jsonContent = convertToJson(readContent); | |
| } catch (err) { | |
| return Promise.reject(err); | |
| } | |
| // Attempt to write the JSON content to file | |
| try { | |
| await writeFile(writePath, JSON.stringify(jsonContent)); | |
| } catch (err) { | |
| return Promise.reject(err); | |
| } | |
| // Attempt to read the file | |
| try { | |
| readContent = await readFile(readPath, 'utf-8'); | |
| } catch (err) { | |
| throw err; | |
| } | |
| // Attempt to convert content to JSON | |
| try { | |
| jsonContent = convertToJson(readContent); | |
| } catch (err) { | |
| throw err; | |
| } | |
| // Attempt to write the JSON content to file | |
| try { | |
| await writeFile(writePath, JSON.stringify(jsonContent)); | |
| } catch (err) { | |
| throw err; | |
| } |
🤖 Prompt for AI Agents
In npm/scripts/helpers/readAndWriteJson.ts between lines 18 and 37, replace all
instances of returning Promise.reject(err) with simply throwing the error using
throw err. This change aligns the error handling with the sibling file
npm/scripts/utils/readAndWriteJson.ts, ensuring consistent throw semantics in
async functions and preserving stack traces. Additionally, consider
consolidating the two implementations into a single shared helper to avoid
duplication.
Contributor
Author
|
@CodeRabbit please review |
Contributor
|
Tip For best results, initiate chat on the files or code changes. @sagarkori143 Thank you for submitting this comprehensive modularization effort! This is an excellent architectural improvement that brings much better organization to the build process. Here's my detailed review: 🎯 Overall Assessment: Strong ImplementationThis PR demonstrates excellent software engineering practices with clean modularization, proper error handling, and thoughtful architecture. ✅ Strengths1. Excellent Architecture
2. Robust Error Handling// Great approach in npm/index.ts
try {
await runBuildPostList();
} catch (error) {
logger.error(`Error in buildPostList: ${error.message}`);
errorFaced = true;
}3. Proper Logging & Monitoring
4. TypeScript Best Practices
🔧 Suggestions for Enhancement1. Consider Parallel ExecutionThe current sequential execution might be slower than necessary: // Current approach (sequential)
await runBuildPostList();
await runBuildDashboard();
await runBuildTools();
// Potential improvement (parallel where safe)
await Promise.allSettled([
runBuildPostList(),
runBuildDashboard(),
runBuildTools(),
runCaseStudies(),
runBuildNewsroomVideos()
]);2. Enhanced Error ContextConsider adding more context to error messages: catch (error) {
logger.error(`Error in buildPostList: ${error.message}`, {
timestamp: new Date().toISOString(),
task: 'buildPostList',
stack: error.stack
});
}3. Configuration ManagementConsider centralizing configuration: // npm/config/build-config.ts
export const BUILD_CONFIG = {
paths: {
dashboard: '../../config/dashboard.json',
newsroom: '../../config/newsroom_videos.json'
// ... other paths
},
timeouts: {
github: 30000,
youtube: 15000
}
};🧪 Testing Considerations
📚 Documentation SuggestionsConsider adding:
🎉 Notable ImplementationsDashboard Builder (
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (7)
npm/scripts/build-meetings.ts (2)
40-44: Add explicitCALENDAR_IDvalidation (was already flagged).The script still relies on
process.env.CALENDAR_IDwithout checking it first. If the variable is missing the request will silently hit the “primary” calendar or fail with a hard-to-trace 404. Add a guard before the API call and fail fast with a clear message.
51-55: All-day events are still rejected (repeat comment).Google Calendar represents all-day events via
start.date, notstart.dateTime. Throwing here discards perfectly valid events. Usestart.dateTime ?? start.dateinstead and build theDateobject from whichever exists.npm/scripts/tools/extract-tools-github.ts (1)
22-23: Uselesstry/catch— let the original stack propagateThe catch only re-throws, hiding the true stack trace. Simply remove it.
export async function getData(): Promise<ToolsData> { - // eslint-disable-next-line no-useless-catch - try { if (!process.env.GITHUB_TOKEN) { throw new Error('GITHUB_TOKEN environment variable is required'); } // ... rest of the code ... return result.data.items; - } catch (err) { - throw err; - } }Also applies to: 70-72
npm/scripts/tools/tools-object.ts (3)
20-24: Wrong Fuse key – categories will never match
keys: ['tag']is used but eachcategoryListelement exposesname.
Result:fuse.search()always returns empty → everything falls into the fallback branch.Change to:
const options = { includeScore: true, shouldSort: true, threshold: 0.4, - keys: ['tag'] + keys: ['name'] };
101-102: Fragile SHA extraction
tool.url.split('=')[1]assumes a single?ref=query parameter.
GitHub API can return URLs without that pattern (e.g. for default branch).
Usenew URL(tool.url).searchParams.get('ref') ?? tool.repository.default_branchfor robustness.
125-129: Fallback category may crash
targetCategory = 'Others'butfinalToolsObjectis pre-seeded fromcategoryListonly.
If that list does not contain an Others entry this line will throwTypeError: Cannot read properties of undefined.Either ensure 'Others' exists in the initialization:
// After initializing finalToolsObject from categoryList +if (!finalToolsObject['Others']) { + finalToolsObject['Others'] = { + description: 'Other tools', + toolsList: [] + }; +}Or use a defensive check:
const targetCategory = categorySearch.length ? categorySearch[0].item.name : 'Others'; +if (!finalToolsObject[targetCategory]) { + logger.warn(`Category '${targetCategory}' not found, skipping tool: ${jsonToolFileContent.title}`); + return; +} const { toolsList } = finalToolsObject[targetCategory];npm/scripts/tools/combine-tools.ts (1)
25-47: Global mutable state leaks between runs
finalTools,languageList,technologyList, and their Fuse instances are initialised at module load time.
IfcombineTools()is invoked more than once in the same Node process (e.g. during tests) previous state persists and corrupts subsequent results.Move these initialisations inside
combineToolsto guarantee fresh state per run:-const finalTools: FinalToolsListObject = {}; - -for (const category of categoryList) { - finalTools[category.name] = { - description: category.description, - toolsList: [] - }; -} - -// ... other globals ... -const languageList = [...languagesColor]; -const technologyList = [...technologiesColor]; -let languageFuse = new Fuse(languageList, options); -let technologyFuse = new Fuse(technologyList, options); const combineTools = async ( automatedTools: ToolsListObject, manualTools: ToolsListObject, toolsPath: string, tagsPath: string ): Promise<void> => { + const finalTools: FinalToolsListObject = {}; + + for (const category of categoryList) { + finalTools[category.name] = { + description: category.description, + toolsList: [] + }; + } + + const languageList = [...languagesColor]; + const technologyList = [...technologiesColor]; + let languageFuse = new Fuse(languageList, options); + let technologyFuse = new Fuse(technologyList, options);This will require passing these objects to
getFinalToolandprocessManualToolas parameters.
🧹 Nitpick comments (3)
npm/scripts/build-meetings.ts (3)
22-25: Request read-only scope instead of full access.
https://www.googleapis.com/auth/calendargrants read/write permissions. The build script only reads events, sohttps://www.googleapis.com/auth/calendar.readonlyis sufficient and reduces blast-radius if the key leaks.- scopes: ['https://www.googleapis.com/auth/calendar'], + scopes: ['https://www.googleapis.com/auth/calendar.readonly'],
60-63: Leverage optional chaining for cleaner access.Static analysis rightly points out the verbose boolean “AND” pattern. Optional chaining is shorter and avoids accidental truthiness bugs.
- url: - e.extendedProperties?.private && - `https://github.com/asyncapi/community/issues/${e.extendedProperties.private.ISSUE_ID}`, - banner: e.extendedProperties?.private && e.extendedProperties.private.BANNER, + url: e.extendedProperties?.private?.ISSUE_ID + ? `https://github.com/asyncapi/community/issues/${e.extendedProperties.private.ISSUE_ID}` + : undefined, + banner: e.extendedProperties?.private?.BANNER,
71-71: Ensure output directory exists before writing.
writeFileSyncwill throw if../configis missing. Callfs.mkdirSync(dirname(writePath), { recursive: true })just before writing to make the script idempotent.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
npm/runners/build-tools-runner.ts(1 hunks)npm/scripts/build-meetings.ts(1 hunks)npm/scripts/build-newsroom-videos.ts(1 hunks)npm/scripts/build-tools.ts(1 hunks)npm/scripts/index.ts(1 hunks)npm/scripts/tools/combine-tools.ts(1 hunks)npm/scripts/tools/extract-tools-github.ts(1 hunks)npm/scripts/tools/tools-object.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- npm/runners/build-tools-runner.ts
- npm/scripts/build-newsroom-videos.ts
- npm/scripts/index.ts
- npm/scripts/build-tools.ts
🧰 Additional context used
🧬 Code Graph Analysis (2)
npm/scripts/tools/extract-tools-github.ts (4)
types/scripts/tools.ts (1)
ToolsData(91-91)npm/scripts/helpers/logger.ts (1)
logger(18-18)npm/scripts/helpers/utils.ts (1)
pause(46-46)npm/scripts/utils.ts (1)
pause(47-47)
npm/scripts/tools/tools-object.ts (4)
npm/scripts/tools/categorylist.ts (1)
categoryList(108-108)types/scripts/tools.ts (3)
AsyncAPITool(64-66)ToolsData(91-91)ToolsListObject(101-101)npm/scripts/helpers/utils.ts (1)
convertToJson(46-46)npm/scripts/helpers/logger.ts (1)
logger(18-18)
🪛 Biome (1.9.4)
npm/scripts/tools/extract-tools-github.ts
[error] 71-71: The catch clause that only rethrows the original error is useless.
An unnecessary catch clause can be confusing.
Unsafe fix: Remove the try/catch clause.
(lint/complexity/noUselessCatch)
npm/scripts/build-meetings.ts
[error] 62-62: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 180000ms (1)
- GitHub Check: Lighthouse CI
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (9)
npm/scripts/build-meetings.ts (2)
41-42:CALENDAR_IDstill unchecked – previous feedback unresolved
No validation was added before usingprocess.env.CALENDAR_ID, so an undefined value will reach the API and produce a cryptic 404.
51-55: All-day events will crash the build – handlestart.datefallback
The guard still insists onstart.dateTime, ignoring all-day events that only containstart.date, exactly as flagged earlier.npm/scripts/tools/extract-tools-github.ts (2)
22-23: Remove the no-op try/catchThe catch block simply re-throws, hiding the original stack trace and tripping the
no-useless-catchlinter. Delete the wrapper entirely.Also applies to: 70-72
27-27:Setwill not deduplicate objects coming from different pages
Setuses reference equality; identical items fetched in separate requests will still be treated as distinct objects. Use aMapkeyed by a stable identifier (e.g.repo.full_name + '/' + path) instead.Also applies to: 44-46, 60-62
npm/scripts/tools/combine-tools.ts (1)
25-48: Global mutable state leaks between runs
finalTools, tag lists, and their Fuse instances are initialised at module load time. CallingcombineTools()twice in the same process (e.g. tests) reuses stale state and produces corrupted output. Move these initialisations insidecombineToolsor expose a factory.npm/scripts/dashboard/build-dashboard.ts (4)
171-176: Duplicate of earlier feedback: avoid reassigning thediscussionparameter; create a new variable instead to preserve immutability.
198-199: Duplicate of earlier feedback:discussion.timelineItems.updatedAtis undefined – usediscussion.updatedAt.
285-287: Duplicate of earlier feedback: non-null assertion onissue.labelscan crash. Replace with a safe fallback (issue.labels?.nodes ?? []).
321-321: Duplicate of earlier feedback:new Erroronly accepts one argument. Combine the strings in a template literal.
🧹 Nitpick comments (9)
npm/scripts/build-meetings.ts (3)
22-28: Use read-only calendar scope to adhere to least-privilege principleThe script only reads events, yet it requests full-access scope (
auth/calendar). Switching to the read-only scope reduces blast-radius if the token leaks.- scopes: ['https://www.googleapis.com/auth/calendar'], + scopes: ['https://www.googleapis.com/auth/calendar.readonly'],
60-64: Optional chaining & nullish checks can be simplifiedStatic analysis rightly points out redundant truthiness checks. A concise pattern also avoids creating the URL when
ISSUE_IDis absent.- url: - e.extendedProperties?.private && - `https://github.com/asyncapi/community/issues/${e.extendedProperties.private.ISSUE_ID}`, - banner: e.extendedProperties?.private && e.extendedProperties.private.BANNER, + url: e.extendedProperties?.private?.ISSUE_ID + ? `https://github.com/asyncapi/community/issues/${e.extendedProperties.private.ISSUE_ID}` + : undefined, + banner: e.extendedProperties?.private?.BANNER,
71-71: Ensure target directory exists beforewriteFileSyncIf the
config/directory is missing in fresh environments the write will throw. Create the directory (with{recursive:true}) first.- writeFileSync(writePath, eventsForHuman); + mkdirSync(dirname(writePath), { recursive: true }); + writeFileSync(writePath, eventsForHuman);npm/scripts/tools/extract-tools-github.ts (1)
32-33: Nit: parameter casing
PerPageis capitalised. PreferperPageto keep camel-case consistency.npm/scripts/tools/combine-tools.ts (2)
71-115: Repeatedly rebuilding Fuse indices hurts performanceEach time a new tag is added the code reinstantiates
Fuse, turning an O(n) operation into O(n²) for large tag sets.
ConsiderlanguageFuse.add(doc)/technologyFuse.add(doc)(Fuse supports incremental indexing) or perform a single rebuild after all additions.Also applies to: 118-140
75-77: Unnecessaryawaiton synchronousFuse.search
searchis synchronous; theawaitadds noise and suggests IO where none exists.Also applies to: 96-97, 121-122
npm/scripts/dashboard/build-dashboard.ts (3)
78-82: Centralize GitHub token resolution
getDiscussions(andgetDiscussionByIDbelow) re-reads and validatesprocess.env.GITHUB_TOKENon every recursive/iterative call. This happens dozens of times during a single dashboard build and adds avoidable overhead.Extract the token once at module scope and throw immediately if it’s missing:
-const token = process.env.GITHUB_TOKEN; -if (!token) { - throw new Error('GitHub token is not set in environment variables'); -} +const GITHUB_TOKEN = process.env.GITHUB_TOKEN; +if (!GITHUB_TOKEN) { + throw new Error('GITHUB_TOKEN is not set in environment variables'); +}Then pass
GITHUB_TOKENto both GraphQL calls.
This keeps the hot-path tight and removes duplicated checks.
36-42: Month calculation is approximate
monthsSinceassumes every month is 30 days. While acceptable for a rough score, document the approximation explicitly or switch to a calendar-aware diff (date-fns/luxon) if accuracy matters for ranking.
316-318: Prefer logger overconsole.logThe script consistently uses
loggerfor structured logs except here:console.log("build dashboard main script has been done successfully")Use
logger.info(...)for consistency and log formatting.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
npm/runners/build-dashboard-runner.ts(1 hunks)npm/runners/build-newsroom-videos-runner.ts(1 hunks)npm/runners/build-post-list-runner.ts(1 hunks)npm/runners/build-tools-runner.ts(1 hunks)npm/runners/case-studies-runner.ts(1 hunks)npm/scripts/build-meetings.ts(1 hunks)npm/scripts/build-newsroom-videos.ts(1 hunks)npm/scripts/build-tools.ts(1 hunks)npm/scripts/dashboard/build-dashboard.ts(1 hunks)npm/scripts/tools/combine-tools.ts(1 hunks)npm/scripts/tools/extract-tools-github.ts(1 hunks)npm/scripts/tools/tools-object.ts(1 hunks)npm/scripts/utils/logger.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- npm/scripts/utils/logger.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- npm/runners/build-dashboard-runner.ts
- npm/runners/build-tools-runner.ts
- npm/runners/build-newsroom-videos-runner.ts
- npm/scripts/build-newsroom-videos.ts
- npm/runners/build-post-list-runner.ts
- npm/runners/case-studies-runner.ts
- npm/scripts/build-tools.ts
- npm/scripts/tools/tools-object.ts
🧰 Additional context used
🧬 Code Graph Analysis (2)
npm/scripts/tools/combine-tools.ts (5)
types/scripts/tools.ts (5)
FinalToolsListObject(102-102)AsyncAPITool(64-66)FinalAsyncAPITool(68-71)LanguageColorItem(34-38)ToolsListObject(101-101)npm/scripts/tools/categorylist.ts (1)
categoryList(108-108)npm/scripts/tools/tags-color.ts (2)
languagesColor(175-175)technologiesColor(175-175)npm/scripts/utils/logger.ts (1)
logger(18-18)npm/scripts/tools/tools-object.ts (1)
createToolObject(155-155)
npm/scripts/tools/extract-tools-github.ts (3)
types/scripts/tools.ts (1)
ToolsData(91-91)npm/scripts/utils/logger.ts (1)
logger(18-18)npm/scripts/helpers/utils.ts (1)
pause(46-46)
🪛 Biome (1.9.4)
npm/scripts/build-meetings.ts
[error] 62-62: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
npm/scripts/tools/extract-tools-github.ts
[error] 71-71: The catch clause that only rethrows the original error is useless.
An unnecessary catch clause can be confusing.
Unsafe fix: Remove the try/catch clause.
(lint/complexity/noUselessCatch)
⏰ Context from checks skipped due to timeout of 180000ms (1)
- GitHub Check: Test NodeJS PR - macos-13
sagarkori143
changed the title
sagarkori143
changed the title
Contributor
Author
|
@CodeRabbit please review the index.ts file in the npm folder |
Contributor
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 1655 @sagarkori143 Here's my detailed review of the 🎯 Overall Structure: Good FoundationThe file demonstrates solid architectural thinking with clear separation of concerns and proper error isolation. ✅ Strengths
🔧 Issues & Suggestions1. TypeScript Type Error// ❌ Current (incorrect)
let errorFaced: Boolean = false;
// ✅ Improved (use primitive boolean)
let errorFaced: boolean = false;2. Inconsistent Error Handling// ❌ Current (mixed logging approaches)
logger.error('Error building posts: ', err as Error);
console.log("Some scripts faced error while running...");
// ✅ Improved (consistent logging)
logger.error('Error building posts:', err);
logger.info('Successfully executed all build scripts');3. Problematic Error Constructor// ❌ Current (incorrect Error syntax)
throw new Error('Error executing build scripts: ', error as Error);
// ✅ Improved (correct syntax)
throw new Error(`Error executing build scripts: ${error}`);4. Enhanced Error Context// ✅ Better error logging with context
try {
await runBuildPostList();
} catch (err) {
errorFaced = true;
logger.error('Error building posts:', {
error: err,
task: 'buildPostList',
timestamp: new Date().toISOString()
});
}5. Consider Parallel Execution// 🚀 Performance improvement option
const buildTasks = [
{ name: 'posts', task: runBuildPostList },
{ name: 'dashboard', task: runBuildDashboard },
{ name: 'tools', task: runBuildTools },
{ name: 'caseStudies', task: runCaseStudies },
{ name: 'newsroomVideos', task: runBuildNewsroomVideos }
];
const results = await Promise.allSettled(
buildTasks.map(({ task }) => task())
);
// Process results and log errors
results.forEach((result, index) => {
if (result.status === 'rejected') {
errorFaced = true;
logger.error(`Error building ${buildTasks[index].name}:`, result.reason);
}
});📝 Improved VersionHere's a refined version addressing the key issues: import { runBuildPostList } from './runners/build-post-list-runner';
import { runBuildDashboard } from './runners/build-dashboard-runner';
import { runBuildTools } from './runners/build-tools-runner';
import { runCaseStudies } from './runners/case-studies-runner';
import { runBuildNewsroomVideos } from './runners/build-newsroom-videos-runner';
import { logger } from '../scripts/helpers/logger';
async function main(): Promise<void> {
let errorFaced: boolean = false;
try {
// Build Post List
try {
await runBuildPostList();
logger.info('Successfully built post list');
} catch (err) {
errorFaced = true;
logger.error('Error building posts:', err);
}
// Build Dashboard
try {
await runBuildDashboard();
logger.info('Successfully built dashboard');
} catch (err) {
errorFaced = true;
logger.error('Error building dashboard:', err);
}
// Build Tools
try {
await runBuildTools();
logger.info('Successfully built tools');
} catch (err) {
errorFaced = true;
logger.error('Error building tools:', err);
}
// Build Case Studies
try {
await runCaseStudies();
logger.info('Successfully built case studies');
} catch (err) {
errorFaced = true;
logger.error('Error building case studies:', err);
}
// Build Newsroom Videos
try {
await runBuildNewsroomVideos();
logger.info('Successfully built newsroom videos');
} catch (err) {
errorFaced = true;
logger.error('Error building newsroom videos:', err);
}
// Final status
if (errorFaced) {
logger.warn('Some build tasks encountered errors. Please check the logs above for details.');
process.exit(1);
} else {
logger.info('✅ Successfully executed all build scripts');
}
} catch (error) {
logger.error('Unexpected error during build execution:', error);
process.exit(1);
}
}
main().catch((error) => {
logger.error('Fatal error in main execution:', error);
process.exit(1);
});🎯 SummaryThe current implementation is functionally solid but has some TypeScript and consistency issues. The suggested improvements focus on:
Overall, this is a good foundation that just needs some polish to be production-ready! 🚀 |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
npm/runners/build-post-list-runner.ts (1)
70-79: Add missing test environment check for consistency.This runner is missing the conditional execution check that all other runners have. For architectural consistency, add the test environment check:
+// Run only in non-test environments +if (process.env.NODE_ENV === 'test') { + logger.info('Skipping post list build in test environment'); +} else { // Self-executing async function to handle top-level await (async () => { try { await runBuildPostList(); } catch (error) { // Ensure we exit with error code process.exit(1); } })(); +} + +// Export for testing purposes +export { runBuildPostList };This ensures consistency with the established pattern across all runner modules.
🧹 Nitpick comments (1)
scripts/helpers/logger.ts (1)
53-55: Optimize performance by avoiding spread operator in reduce.The spread operator in the reduce accumulator causes O(n²) time complexity. Use direct property assignment for better performance.
- const filteredMeta = Object.entries(meta) - .filter(([key]) => !excludeFields.includes(key)) - .reduce((acc, [key, value]) => ({ ...acc, [key]: value }), {}); + const filteredMeta = Object.entries(meta) + .filter(([key]) => !excludeFields.includes(key)) + .reduce((acc, [key, value]) => { + acc[key] = value; + return acc; + }, {} as Record<string, unknown>);
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
npm/runners/build-adopters-list-runner.ts(1 hunks)npm/runners/build-dashboard-runner.ts(1 hunks)npm/runners/build-finance-info-list-runner.ts(1 hunks)npm/runners/build-meetings-runner.ts(1 hunks)npm/runners/build-newsroom-videos-runner.ts(1 hunks)npm/runners/build-pages-runner.ts(1 hunks)npm/runners/build-post-list-runner.ts(1 hunks)npm/runners/build-rss-runner.ts(1 hunks)npm/runners/build-tools-runner.ts(1 hunks)npm/runners/case-studies-runner.ts(1 hunks)npm/runners/compose-blog-runner.ts(1 hunks)scripts/build-newsroom-videos.ts(4 hunks)scripts/helpers/logger.ts(1 hunks)types/errors/ApiError.ts(1 hunks)types/errors/CustomError.ts(1 hunks)types/errors/RunnerError.ts(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- types/errors/ApiError.ts
- types/errors/RunnerError.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- scripts/build-newsroom-videos.ts
🧰 Additional context used
🧠 Learnings (14)
📓 Common learnings
Learnt from: sagarkori143
PR: asyncapi/website#4192
File: npm/runners/build-newsroom-videos-runner.ts:8-15
Timestamp: 2025-06-19T13:49:29.796Z
Learning: In the AsyncAPI website modularization project, error handling is centralized in the top-level orchestrator function (npm/index.ts) with comprehensive logging and context. Individual runner functions in npm/runners/ are kept simple and let errors propagate naturally to the centralized handler, avoiding redundant try-catch blocks that only rethrow errors.
Learnt from: sagarkori143
PR: asyncapi/website#4192
File: npm/runners/build-pages-runner.ts:7-14
Timestamp: 2025-06-19T13:51:27.459Z
Learning: In the AsyncAPI website build system, individual runner functions use try-catch blocks that rethrow errors to maintain consistent error boundaries, while the top-level orchestrator in npm/index.ts provides meaningful error logging with task-specific context using logger.error(`Error building ${taskName}:`, error). This separation of concerns allows for clean error handling at different architectural layers.
Learnt from: sagarkori143
PR: asyncapi/website#0
File: :0-0
Timestamp: 2025-06-20T14:47:22.389Z
Learning: The AsyncAPI website modularization PR uses Promise.allSettled for parallel execution of build tasks, which provides better performance and fault isolation compared to sequential execution.
Learnt from: anshgoyalevil
PR: asyncapi/website#3950
File: scripts/utils/check-locales.ts:122-129
Timestamp: 2025-04-20T16:05:16.482Z
Learning: In the AsyncAPI website project, Next.js throws errors at runtime when locale files are missing, making additional validation for missing files unnecessary in the check-locales script.
Learnt from: akshatnema
PR: asyncapi/website#3265
File: tests/fixtures/toolsObjectData.js:51-52
Timestamp: 2024-10-09T17:35:36.557Z
Learning: When reviewing code in the 'asyncapi/website' repository, akshatnema prefers that I do not provide committable code suggestions.
Learnt from: anshgoyalevil
PR: asyncapi/website#3557
File: scripts/markdown/check-editlinks.js:58-59
Timestamp: 2025-01-08T15:15:00.759Z
Learning: In the AsyncAPI codebase, batch processing operations (like in the Dashboard script and check-editlinks.js) follow a sequential pattern using await in loops, which is the preferred approach for maintaining consistency across the codebase.
Learnt from: akshatnema
PR: asyncapi/website#3378
File: scripts/markdown/check-markdown.js:1-1
Timestamp: 2024-11-25T18:34:51.303Z
Learning: When reviewing `scripts/markdown/check-markdown.js`, optimizations should be addressed in separate issues and not included in the current pull request.
Learnt from: asyncapi-bot
PR: asyncapi/website#0
File: :0-0
Timestamp: 2025-02-18T12:07:42.211Z
Learning: The following PR commands are supported in the asyncapi/website repository:
- `/please-take-a-look` or `/ptal`: Requests attention from reviewers who haven't reviewed the PR
- `/ready-to-merge` or `/rtm`: Triggers automerge when all conditions are met
- `/do-not-merge` or `/dnm`: Blocks automerge even if all conditions are met
- `/autoupdate` or `/au`: Adds autoupdate label to keep PR in sync with target branch
- `/update` or `/u`: One-time update of PR with latest changes from target branch
Learnt from: anshgoyalevil
PR: asyncapi/website#3557
File: .github/workflows/check-edit-links.yml:25-29
Timestamp: 2025-01-08T15:16:27.655Z
Learning: In GitHub workflows running scripts with process.exit statements for error handling (like check-editlinks.js in asyncapi/website), avoid adding error suppression (|| true) at the workflow level as it would mask the intended error reporting mechanism.
Learnt from: sagarkori143
PR: asyncapi/website#0
File: :0-0
Timestamp: 2025-06-20T14:47:22.389Z
Learning: Runner files in the npm/runners directory should follow consistent error handling patterns, throwing new Error instances with descriptive messages rather than just rethrowing caught errors.
Learnt from: akshatnema
PR: asyncapi/website#3265
File: scripts/tools/tools-object.js:93-98
Timestamp: 2024-11-29T19:42:31.175Z
Learning: In `scripts/tools/tools-object.js`, when validation fails in the `convertTools` function, errors should be thrown or rejected instead of only logging them, to ensure proper error propagation and handling.
Learnt from: vishvamsinh28
PR: asyncapi/website#3284
File: scripts/build-post-list.js:49-53
Timestamp: 2024-11-10T18:16:22.273Z
Learning: In `scripts/build-post-list.js`, the `walkDirectories` function depends on sequential execution because it mutates `resultObj`. Parallelizing the loop with `Promise.all` causes tests to fail.
npm/runners/build-dashboard-runner.ts (12)
Learnt from: sagarkori143
PR: asyncapi/website#4192
File: npm/runners/build-pages-runner.ts:7-14
Timestamp: 2025-06-19T13:51:27.459Z
Learning: In the AsyncAPI website build system, individual runner functions use try-catch blocks that rethrow errors to maintain consistent error boundaries, while the top-level orchestrator in npm/index.ts provides meaningful error logging with task-specific context using logger.error(`Error building ${taskName}:`, error). This separation of concerns allows for clean error handling at different architectural layers.
Learnt from: sagarkori143
PR: asyncapi/website#0
File: :0-0
Timestamp: 2025-06-20T14:47:22.389Z
Learning: Runner files in the npm/runners directory should follow consistent error handling patterns, throwing new Error instances with descriptive messages rather than just rethrowing caught errors.
Learnt from: sagarkori143
PR: asyncapi/website#4192
File: npm/runners/build-newsroom-videos-runner.ts:8-15
Timestamp: 2025-06-19T13:49:29.796Z
Learning: In the AsyncAPI website modularization project, error handling is centralized in the top-level orchestrator function (npm/index.ts) with comprehensive logging and context. Individual runner functions in npm/runners/ are kept simple and let errors propagate naturally to the centralized handler, avoiding redundant try-catch blocks that only rethrow errors.
Learnt from: sagarkori143
PR: asyncapi/website#0
File: :0-0
Timestamp: 2025-06-20T14:47:22.389Z
Learning: The AsyncAPI website modularization PR uses Promise.allSettled for parallel execution of build tasks, which provides better performance and fault isolation compared to sequential execution.
Learnt from: akshatnema
PR: asyncapi/website#3423
File: tests/index.test.js:2-7
Timestamp: 2025-01-18T08:44:43.614Z
Learning: In the AsyncAPI website project, JavaScript test files must include the .ts extension when importing TypeScript files (e.g., `require('../scripts/build-rss.ts')`). This is enforced by the project's configuration which uses `moduleResolution: "bundler"` in tsconfig.json and TypeScript-aware ESLint plugins. The .ts extensions are required even though the files are imported using CommonJS require statements.
Learnt from: akshatnema
PR: asyncapi/website#3265
File: scripts/tools/tools-object.js:93-98
Timestamp: 2024-11-29T19:42:31.175Z
Learning: In `scripts/tools/tools-object.js`, when validation fails in the `convertTools` function, errors should be thrown or rejected instead of only logging them, to ensure proper error propagation and handling.
Learnt from: akshatnema
PR: asyncapi/website#3265
File: scripts/tools/tools-object.js:49-51
Timestamp: 2024-11-01T11:40:51.617Z
Learning: In JavaScript code, prefer using a custom LogService with Winston for logging errors instead of `console.error` or ad-hoc logging.
Learnt from: anshgoyalevil
PR: asyncapi/website#3557
File: .github/workflows/check-edit-links.yml:25-29
Timestamp: 2025-01-08T15:16:27.655Z
Learning: In GitHub workflows running scripts with process.exit statements for error handling (like check-editlinks.js in asyncapi/website), avoid adding error suppression (|| true) at the workflow level as it would mask the intended error reporting mechanism.
Learnt from: JeelRajodiya
PR: asyncapi/website#3423
File: scripts/index.ts:29-31
Timestamp: 2025-03-16T15:55:44.257Z
Learning: In the AsyncAPI codebase, adding try-catch blocks around function calls is unnecessary when the only purpose is to add context to the error message, as errors will be thrown from the functions themselves with full stack traces that provide adequate context. Try-catch blocks should only be added when there's a need for graceful degradation, partial success behavior, or specific error handling logic.
Learnt from: JeelRajodiya
PR: asyncapi/website#3423
File: scripts/adopters/index.ts:9-11
Timestamp: 2024-12-31T09:25:52.573Z
Learning: The `writeJSON` function already includes error handling, so additional try/catch blocks around it may be redundant.
Learnt from: anshgoyalevil
PR: asyncapi/website#3301
File: scripts/markdown/check-markdown.js:0-0
Timestamp: 2024-10-18T17:24:45.053Z
Learning: In `scripts/markdown/check-markdown.js`, the script is not run in an asynchronous environment, so refactoring it to use `async/await` is unnecessary.
Learnt from: vishvamsinh28
PR: asyncapi/website#3284
File: scripts/build-post-list.js:49-53
Timestamp: 2024-11-10T18:16:22.273Z
Learning: In `scripts/build-post-list.js`, the `walkDirectories` function depends on sequential execution because it mutates `resultObj`. Parallelizing the loop with `Promise.all` causes tests to fail.
npm/runners/build-adopters-list-runner.ts (16)
Learnt from: sagarkori143
PR: asyncapi/website#4192
File: npm/runners/build-pages-runner.ts:7-14
Timestamp: 2025-06-19T13:51:27.459Z
Learning: In the AsyncAPI website build system, individual runner functions use try-catch blocks that rethrow errors to maintain consistent error boundaries, while the top-level orchestrator in npm/index.ts provides meaningful error logging with task-specific context using logger.error(`Error building ${taskName}:`, error). This separation of concerns allows for clean error handling at different architectural layers.
Learnt from: sagarkori143
PR: asyncapi/website#0
File: :0-0
Timestamp: 2025-06-20T14:47:22.389Z
Learning: Runner files in the npm/runners directory should follow consistent error handling patterns, throwing new Error instances with descriptive messages rather than just rethrowing caught errors.
Learnt from: sagarkori143
PR: asyncapi/website#4192
File: npm/runners/build-newsroom-videos-runner.ts:8-15
Timestamp: 2025-06-19T13:49:29.796Z
Learning: In the AsyncAPI website modularization project, error handling is centralized in the top-level orchestrator function (npm/index.ts) with comprehensive logging and context. Individual runner functions in npm/runners/ are kept simple and let errors propagate naturally to the centralized handler, avoiding redundant try-catch blocks that only rethrow errors.
Learnt from: vishvamsinh28
PR: asyncapi/website#3284
File: scripts/build-post-list.js:49-53
Timestamp: 2024-11-10T18:16:22.273Z
Learning: In `scripts/build-post-list.js`, the `walkDirectories` function depends on sequential execution because it mutates `resultObj`. Parallelizing the loop with `Promise.all` causes tests to fail.
Learnt from: akshatnema
PR: asyncapi/website#3101
File: scripts/build-rss.js:4-6
Timestamp: 2024-11-01T09:35:23.912Z
Learning: Converting `getAllPosts()` to asynchronous in `scripts/build-rss.js` causes the existing build system to fail.
Learnt from: sagarkori143
PR: asyncapi/website#0
File: :0-0
Timestamp: 2025-06-20T14:47:22.389Z
Learning: The AsyncAPI website modularization PR uses Promise.allSettled for parallel execution of build tasks, which provides better performance and fault isolation compared to sequential execution.
Learnt from: akshatnema
PR: asyncapi/website#3423
File: tests/index.test.js:2-7
Timestamp: 2025-01-18T08:44:43.614Z
Learning: In the AsyncAPI website project, JavaScript test files must include the .ts extension when importing TypeScript files (e.g., `require('../scripts/build-rss.ts')`). This is a project-specific requirement enforced by the linting rules and build setup, even though it differs from typical Node.js behavior.
Learnt from: akshatnema
PR: asyncapi/website#3423
File: tests/index.test.js:2-7
Timestamp: 2025-01-18T08:44:43.614Z
Learning: In the AsyncAPI website project, JavaScript test files must include the .ts extension when importing TypeScript files (e.g., `require('../scripts/build-rss.ts')`). This is enforced by the project's configuration which uses `moduleResolution: "bundler"` in tsconfig.json and TypeScript-aware ESLint plugins. The .ts extensions are required even though the files are imported using CommonJS require statements.
Learnt from: JeelRajodiya
PR: asyncapi/website#3423
File: scripts/index.ts:29-31
Timestamp: 2025-03-16T15:55:44.257Z
Learning: In the AsyncAPI codebase, adding try-catch blocks around function calls is unnecessary when the only purpose is to add context to the error message, as errors will be thrown from the functions themselves with full stack traces that provide adequate context. Try-catch blocks should only be added when there's a need for graceful degradation, partial success behavior, or specific error handling logic.
Learnt from: JeelRajodiya
PR: asyncapi/website#3423
File: scripts/adopters/index.ts:9-11
Timestamp: 2024-12-31T09:25:52.573Z
Learning: The `writeJSON` function already includes error handling, so additional try/catch blocks around it may be redundant.
Learnt from: anshgoyalevil
PR: asyncapi/website#3301
File: scripts/markdown/check-markdown.js:0-0
Timestamp: 2024-10-18T17:24:45.053Z
Learning: In `scripts/markdown/check-markdown.js`, the script is not run in an asynchronous environment, so refactoring it to use `async/await` is unnecessary.
Learnt from: anshgoyalevil
PR: asyncapi/website#3950
File: scripts/utils/check-locales.ts:122-129
Timestamp: 2025-04-20T16:05:16.482Z
Learning: In the AsyncAPI website project, Next.js throws errors at runtime when locale files are missing, making additional validation for missing files unnecessary in the check-locales script.
Learnt from: anshgoyalevil
PR: asyncapi/website#3557
File: .github/workflows/check-edit-links.yml:25-29
Timestamp: 2025-01-08T15:16:27.655Z
Learning: In GitHub workflows running scripts with process.exit statements for error handling (like check-editlinks.js in asyncapi/website), avoid adding error suppression (|| true) at the workflow level as it would mask the intended error reporting mechanism.
Learnt from: akshatnema
PR: asyncapi/website#3265
File: scripts/tools/tools-object.js:93-98
Timestamp: 2024-11-29T19:42:31.175Z
Learning: In `scripts/tools/tools-object.js`, when validation fails in the `convertTools` function, errors should be thrown or rejected instead of only logging them, to ensure proper error propagation and handling.
Learnt from: anshgoyalevil
PR: asyncapi/website#3557
File: scripts/markdown/check-editlinks.js:58-59
Timestamp: 2025-01-08T15:15:00.759Z
Learning: In the AsyncAPI codebase, batch processing operations (like in the Dashboard script and check-editlinks.js) follow a sequential pattern using await in loops, which is the preferred approach for maintaining consistency across the codebase.
Learnt from: vishvamsinh28
PR: asyncapi/website#3284
File: scripts/build-post-list.js:28-46
Timestamp: 2024-11-10T18:16:35.551Z
Learning: In the `scripts/build-post-list.js` JavaScript file, tests rely on the `result` object being a global variable. Moving it inside the `buildPostList` function causes tests to fail; therefore, keep `result` as a global variable.
npm/runners/build-finance-info-list-runner.ts (11)
Learnt from: sagarkori143
PR: asyncapi/website#4192
File: npm/runners/build-pages-runner.ts:7-14
Timestamp: 2025-06-19T13:51:27.459Z
Learning: In the AsyncAPI website build system, individual runner functions use try-catch blocks that rethrow errors to maintain consistent error boundaries, while the top-level orchestrator in npm/index.ts provides meaningful error logging with task-specific context using logger.error(`Error building ${taskName}:`, error). This separation of concerns allows for clean error handling at different architectural layers.
Learnt from: vishvamsinh28
PR: asyncapi/website#3284
File: scripts/build-post-list.js:49-53
Timestamp: 2024-11-10T18:16:22.273Z
Learning: In `scripts/build-post-list.js`, the `walkDirectories` function depends on sequential execution because it mutates `resultObj`. Parallelizing the loop with `Promise.all` causes tests to fail.
Learnt from: sagarkori143
PR: asyncapi/website#0
File: :0-0
Timestamp: 2025-06-20T14:47:22.389Z
Learning: Runner files in the npm/runners directory should follow consistent error handling patterns, throwing new Error instances with descriptive messages rather than just rethrowing caught errors.
Learnt from: sagarkori143
PR: asyncapi/website#4192
File: npm/runners/build-newsroom-videos-runner.ts:8-15
Timestamp: 2025-06-19T13:49:29.796Z
Learning: In the AsyncAPI website modularization project, error handling is centralized in the top-level orchestrator function (npm/index.ts) with comprehensive logging and context. Individual runner functions in npm/runners/ are kept simple and let errors propagate naturally to the centralized handler, avoiding redundant try-catch blocks that only rethrow errors.
Learnt from: sagarkori143
PR: asyncapi/website#0
File: :0-0
Timestamp: 2025-06-20T14:47:22.389Z
Learning: The AsyncAPI website modularization PR uses Promise.allSettled for parallel execution of build tasks, which provides better performance and fault isolation compared to sequential execution.
Learnt from: akshatnema
PR: asyncapi/website#3101
File: scripts/build-rss.js:4-6
Timestamp: 2024-11-01T09:35:23.912Z
Learning: Converting `getAllPosts()` to asynchronous in `scripts/build-rss.js` causes the existing build system to fail.
Learnt from: akshatnema
PR: asyncapi/website#3265
File: scripts/tools/tools-object.js:93-98
Timestamp: 2024-11-29T19:42:31.175Z
Learning: In `scripts/tools/tools-object.js`, when validation fails in the `convertTools` function, errors should be thrown or rejected instead of only logging them, to ensure proper error propagation and handling.
Learnt from: anshgoyalevil
PR: asyncapi/website#3557
File: .github/workflows/check-edit-links.yml:25-29
Timestamp: 2025-01-08T15:16:27.655Z
Learning: In GitHub workflows running scripts with process.exit statements for error handling (like check-editlinks.js in asyncapi/website), avoid adding error suppression (|| true) at the workflow level as it would mask the intended error reporting mechanism.
Learnt from: anshgoyalevil
PR: asyncapi/website#3301
File: scripts/markdown/check-markdown.js:0-0
Timestamp: 2024-10-18T17:24:45.053Z
Learning: In `scripts/markdown/check-markdown.js`, the script is not run in an asynchronous environment, so refactoring it to use `async/await` is unnecessary.
Learnt from: anshgoyalevil
PR: asyncapi/website#3950
File: scripts/utils/check-locales.ts:122-129
Timestamp: 2025-04-20T16:05:16.482Z
Learning: In the AsyncAPI website project, Next.js throws errors at runtime when locale files are missing, making additional validation for missing files unnecessary in the check-locales script.
Learnt from: anshgoyalevil
PR: asyncapi/website#3557
File: scripts/markdown/check-editlinks.js:58-59
Timestamp: 2025-01-08T15:15:00.759Z
Learning: In the AsyncAPI codebase, batch processing operations (like in the Dashboard script and check-editlinks.js) follow a sequential pattern using await in loops, which is the preferred approach for maintaining consistency across the codebase.
npm/runners/compose-blog-runner.ts (16)
Learnt from: sagarkori143
PR: asyncapi/website#4192
File: npm/runners/build-pages-runner.ts:7-14
Timestamp: 2025-06-19T13:51:27.459Z
Learning: In the AsyncAPI website build system, individual runner functions use try-catch blocks that rethrow errors to maintain consistent error boundaries, while the top-level orchestrator in npm/index.ts provides meaningful error logging with task-specific context using logger.error(`Error building ${taskName}:`, error). This separation of concerns allows for clean error handling at different architectural layers.
Learnt from: sagarkori143
PR: asyncapi/website#4192
File: npm/runners/build-newsroom-videos-runner.ts:8-15
Timestamp: 2025-06-19T13:49:29.796Z
Learning: In the AsyncAPI website modularization project, error handling is centralized in the top-level orchestrator function (npm/index.ts) with comprehensive logging and context. Individual runner functions in npm/runners/ are kept simple and let errors propagate naturally to the centralized handler, avoiding redundant try-catch blocks that only rethrow errors.
Learnt from: sagarkori143
PR: asyncapi/website#0
File: :0-0
Timestamp: 2025-06-20T14:47:22.389Z
Learning: Runner files in the npm/runners directory should follow consistent error handling patterns, throwing new Error instances with descriptive messages rather than just rethrowing caught errors.
Learnt from: akshatnema
PR: asyncapi/website#3101
File: scripts/build-rss.js:4-6
Timestamp: 2024-11-01T09:35:23.912Z
Learning: Converting `getAllPosts()` to asynchronous in `scripts/build-rss.js` causes the existing build system to fail.
Learnt from: akshatnema
PR: asyncapi/website#3423
File: tests/index.test.js:2-7
Timestamp: 2025-01-18T08:44:43.614Z
Learning: In the AsyncAPI website project, JavaScript test files must include the .ts extension when importing TypeScript files (e.g., `require('../scripts/build-rss.ts')`). This is a project-specific requirement enforced by the linting rules and build setup, even though it differs from typical Node.js behavior.
Learnt from: akshatnema
PR: asyncapi/website#3423
File: tests/index.test.js:2-7
Timestamp: 2025-01-18T08:44:43.614Z
Learning: In the AsyncAPI website project, JavaScript test files must include the .ts extension when importing TypeScript files (e.g., `require('../scripts/build-rss.ts')`). This is enforced by the project's configuration which uses `moduleResolution: "bundler"` in tsconfig.json and TypeScript-aware ESLint plugins. The .ts extensions are required even though the files are imported using CommonJS require statements.
Learnt from: akshatnema
PR: asyncapi/website#3423
File: tests/index.test.js:2-7
Timestamp: 2025-01-18T08:44:43.614Z
Learning: In JavaScript test files within the AsyncAPI website project, TypeScript file imports must include the .ts extension to avoid lint errors, even though the files being imported are JavaScript files.
Learnt from: akshatnema
PR: asyncapi/website#3378
File: scripts/markdown/check-markdown.js:1-1
Timestamp: 2024-11-25T18:34:51.303Z
Learning: When reviewing `scripts/markdown/check-markdown.js`, optimizations should be addressed in separate issues and not included in the current pull request.
Learnt from: anshgoyalevil
PR: asyncapi/website#3950
File: scripts/utils/check-locales.ts:122-129
Timestamp: 2025-04-20T16:05:16.482Z
Learning: In the AsyncAPI website project, Next.js throws errors at runtime when locale files are missing, making additional validation for missing files unnecessary in the check-locales script.
Learnt from: anshgoyalevil
PR: asyncapi/website#3557
File: .github/workflows/check-edit-links.yml:25-29
Timestamp: 2025-01-08T15:16:27.655Z
Learning: In GitHub workflows running scripts with process.exit statements for error handling (like check-editlinks.js in asyncapi/website), avoid adding error suppression (|| true) at the workflow level as it would mask the intended error reporting mechanism.
Learnt from: JeelRajodiya
PR: asyncapi/website#3423
File: scripts/index.ts:29-31
Timestamp: 2025-03-16T15:55:44.257Z
Learning: In the AsyncAPI codebase, adding try-catch blocks around function calls is unnecessary when the only purpose is to add context to the error message, as errors will be thrown from the functions themselves with full stack traces that provide adequate context. Try-catch blocks should only be added when there's a need for graceful degradation, partial success behavior, or specific error handling logic.
Learnt from: akshatnema
PR: asyncapi/website#3265
File: scripts/tools/tools-object.js:93-98
Timestamp: 2024-11-29T19:42:31.175Z
Learning: In `scripts/tools/tools-object.js`, when validation fails in the `convertTools` function, errors should be thrown or rejected instead of only logging them, to ensure proper error propagation and handling.
Learnt from: JeelRajodiya
PR: asyncapi/website#3423
File: scripts/adopters/index.ts:9-11
Timestamp: 2024-12-31T09:25:52.573Z
Learning: The `writeJSON` function already includes error handling, so additional try/catch blocks around it may be redundant.
Learnt from: vishvamsinh28
PR: asyncapi/website#3378
File: tests/markdown/check-markdown.test.js:133-138
Timestamp: 2024-11-29T17:36:09.783Z
Learning: When testing the 'main' function in 'check-markdown.test.js', it's acceptable to simply ensure it doesn't throw any errors, as the functions it calls are already tested elsewhere.
Learnt from: akshatnema
PR: asyncapi/website#3262
File: components/navigation/BlogPostItem.tsx:95-119
Timestamp: 2024-10-11T11:32:30.226Z
Learning: In the `BlogPostItem` component (`components/navigation/BlogPostItem.tsx`), nesting `<a>` tags inside the parent `Link` component leads to hydration issues; therefore, we should avoid nesting `<a>` tags inside `Link` components in this component.
Learnt from: vishvamsinh28
PR: asyncapi/website#3284
File: scripts/build-post-list.js:49-53
Timestamp: 2024-11-10T18:16:22.273Z
Learning: In `scripts/build-post-list.js`, the `walkDirectories` function depends on sequential execution because it mutates `resultObj`. Parallelizing the loop with `Promise.all` causes tests to fail.
scripts/helpers/logger.ts (4)
Learnt from: akshatnema
PR: asyncapi/website#3265
File: scripts/tools/tools-object.js:49-51
Timestamp: 2024-11-01T11:40:51.617Z
Learning: In JavaScript code, prefer using a custom LogService with Winston for logging errors instead of `console.error` or ad-hoc logging.
Learnt from: sagarkori143
PR: asyncapi/website#0
File: :0-0
Timestamp: 2025-06-20T14:47:22.389Z
Learning: Runner files in the npm/runners directory should follow consistent error handling patterns, throwing new Error instances with descriptive messages rather than just rethrowing caught errors.
Learnt from: sagarkori143
PR: asyncapi/website#4192
File: npm/runners/build-pages-runner.ts:7-14
Timestamp: 2025-06-19T13:51:27.459Z
Learning: In the AsyncAPI website build system, individual runner functions use try-catch blocks that rethrow errors to maintain consistent error boundaries, while the top-level orchestrator in npm/index.ts provides meaningful error logging with task-specific context using logger.error(`Error building ${taskName}:`, error). This separation of concerns allows for clean error handling at different architectural layers.
Learnt from: sagarkori143
PR: asyncapi/website#4192
File: npm/runners/build-newsroom-videos-runner.ts:8-15
Timestamp: 2025-06-19T13:49:29.796Z
Learning: In the AsyncAPI website modularization project, error handling is centralized in the top-level orchestrator function (npm/index.ts) with comprehensive logging and context. Individual runner functions in npm/runners/ are kept simple and let errors propagate naturally to the centralized handler, avoiding redundant try-catch blocks that only rethrow errors.
npm/runners/case-studies-runner.ts (13)
Learnt from: sagarkori143
PR: asyncapi/website#4192
File: npm/runners/build-newsroom-videos-runner.ts:8-15
Timestamp: 2025-06-19T13:49:29.796Z
Learning: In the AsyncAPI website modularization project, error handling is centralized in the top-level orchestrator function (npm/index.ts) with comprehensive logging and context. Individual runner functions in npm/runners/ are kept simple and let errors propagate naturally to the centralized handler, avoiding redundant try-catch blocks that only rethrow errors.
Learnt from: sagarkori143
PR: asyncapi/website#0
File: :0-0
Timestamp: 2025-06-20T14:47:22.389Z
Learning: Runner files in the npm/runners directory should follow consistent error handling patterns, throwing new Error instances with descriptive messages rather than just rethrowing caught errors.
Learnt from: sagarkori143
PR: asyncapi/website#4192
File: npm/runners/build-pages-runner.ts:7-14
Timestamp: 2025-06-19T13:51:27.459Z
Learning: In the AsyncAPI website build system, individual runner functions use try-catch blocks that rethrow errors to maintain consistent error boundaries, while the top-level orchestrator in npm/index.ts provides meaningful error logging with task-specific context using logger.error(`Error building ${taskName}:`, error). This separation of concerns allows for clean error handling at different architectural layers.
Learnt from: akshatnema
PR: asyncapi/website#3423
File: tests/index.test.js:2-7
Timestamp: 2025-01-18T08:44:43.614Z
Learning: In the AsyncAPI website project, JavaScript test files must include the .ts extension when importing TypeScript files (e.g., `require('../scripts/build-rss.ts')`). This is enforced by the project's configuration which uses `moduleResolution: "bundler"` in tsconfig.json and TypeScript-aware ESLint plugins. The .ts extensions are required even though the files are imported using CommonJS require statements.
Learnt from: sagarkori143
PR: asyncapi/website#0
File: :0-0
Timestamp: 2025-06-20T14:47:22.389Z
Learning: The AsyncAPI website modularization PR uses Promise.allSettled for parallel execution of build tasks, which provides better performance and fault isolation compared to sequential execution.
Learnt from: anshgoyalevil
PR: asyncapi/website#3557
File: .github/workflows/check-edit-links.yml:25-29
Timestamp: 2025-01-08T15:16:27.655Z
Learning: In GitHub workflows running scripts with process.exit statements for error handling (like check-editlinks.js in asyncapi/website), avoid adding error suppression (|| true) at the workflow level as it would mask the intended error reporting mechanism.
Learnt from: akshatnema
PR: asyncapi/website#3265
File: scripts/tools/tools-object.js:93-98
Timestamp: 2024-11-29T19:42:31.175Z
Learning: In `scripts/tools/tools-object.js`, when validation fails in the `convertTools` function, errors should be thrown or rejected instead of only logging them, to ensure proper error propagation and handling.
Learnt from: akshatnema
PR: asyncapi/website#3265
File: scripts/tools/tools-object.js:49-51
Timestamp: 2024-11-01T11:40:51.617Z
Learning: In JavaScript code, prefer using a custom LogService with Winston for logging errors instead of `console.error` or ad-hoc logging.
Learnt from: JeelRajodiya
PR: asyncapi/website#3423
File: scripts/index.ts:29-31
Timestamp: 2025-03-16T15:55:44.257Z
Learning: In the AsyncAPI codebase, adding try-catch blocks around function calls is unnecessary when the only purpose is to add context to the error message, as errors will be thrown from the functions themselves with full stack traces that provide adequate context. Try-catch blocks should only be added when there's a need for graceful degradation, partial success behavior, or specific error handling logic.
Learnt from: JeelRajodiya
PR: asyncapi/website#3423
File: scripts/adopters/index.ts:9-11
Timestamp: 2024-12-31T09:25:52.573Z
Learning: The `writeJSON` function already includes error handling, so additional try/catch blocks around it may be redundant.
Learnt from: vishvamsinh28
PR: asyncapi/website#3378
File: tests/markdown/check-markdown.test.js:133-138
Timestamp: 2024-11-29T17:36:09.783Z
Learning: When testing the 'main' function in 'check-markdown.test.js', it's acceptable to simply ensure it doesn't throw any errors, as the functions it calls are already tested elsewhere.
Learnt from: anshgoyalevil
PR: asyncapi/website#3950
File: scripts/utils/check-locales.ts:122-129
Timestamp: 2025-04-20T16:05:16.482Z
Learning: In the AsyncAPI website project, Next.js throws errors at runtime when locale files are missing, making additional validation for missing files unnecessary in the check-locales script.
Learnt from: vishvamsinh28
PR: asyncapi/website#3284
File: scripts/build-post-list.js:49-53
Timestamp: 2024-11-10T18:16:22.273Z
Learning: In `scripts/build-post-list.js`, the `walkDirectories` function depends on sequential execution because it mutates `resultObj`. Parallelizing the loop with `Promise.all` causes tests to fail.
npm/runners/build-tools-runner.ts (19)
Learnt from: sagarkori143
PR: asyncapi/website#4192
File: npm/runners/build-pages-runner.ts:7-14
Timestamp: 2025-06-19T13:51:27.459Z
Learning: In the AsyncAPI website build system, individual runner functions use try-catch blocks that rethrow errors to maintain consistent error boundaries, while the top-level orchestrator in npm/index.ts provides meaningful error logging with task-specific context using logger.error(`Error building ${taskName}:`, error). This separation of concerns allows for clean error handling at different architectural layers.
Learnt from: sagarkori143
PR: asyncapi/website#0
File: :0-0
Timestamp: 2025-06-20T14:47:22.389Z
Learning: Runner files in the npm/runners directory should follow consistent error handling patterns, throwing new Error instances with descriptive messages rather than just rethrowing caught errors.
Learnt from: sagarkori143
PR: asyncapi/website#4192
File: npm/runners/build-newsroom-videos-runner.ts:8-15
Timestamp: 2025-06-19T13:49:29.796Z
Learning: In the AsyncAPI website modularization project, error handling is centralized in the top-level orchestrator function (npm/index.ts) with comprehensive logging and context. Individual runner functions in npm/runners/ are kept simple and let errors propagate naturally to the centralized handler, avoiding redundant try-catch blocks that only rethrow errors.
Learnt from: sagarkori143
PR: asyncapi/website#0
File: :0-0
Timestamp: 2025-06-20T14:47:22.389Z
Learning: The AsyncAPI website modularization PR uses Promise.allSettled for parallel execution of build tasks, which provides better performance and fault isolation compared to sequential execution.
Learnt from: akshatnema
PR: asyncapi/website#3423
File: tests/index.test.js:2-7
Timestamp: 2025-01-18T08:44:43.614Z
Learning: In the AsyncAPI website project, JavaScript test files must include the .ts extension when importing TypeScript files (e.g., `require('../scripts/build-rss.ts')`). This is enforced by the project's configuration which uses `moduleResolution: "bundler"` in tsconfig.json and TypeScript-aware ESLint plugins. The .ts extensions are required even though the files are imported using CommonJS require statements.
Learnt from: vishvamsinh28
PR: asyncapi/website#3284
File: scripts/build-post-list.js:49-53
Timestamp: 2024-11-10T18:16:22.273Z
Learning: In `scripts/build-post-list.js`, the `walkDirectories` function depends on sequential execution because it mutates `resultObj`. Parallelizing the loop with `Promise.all` causes tests to fail.
Learnt from: akshatnema
PR: asyncapi/website#3136
File: scripts/tools/combine-tools.js:122-125
Timestamp: 2024-11-01T12:49:32.805Z
Learning: In `scripts/tools/combine-tools.js`, the existing URL parsing logic for `repoUrl` without additional error handling is acceptable and does not require changes.
Learnt from: akshatnema
PR: asyncapi/website#3101
File: scripts/build-rss.js:4-6
Timestamp: 2024-11-01T09:35:23.912Z
Learning: Converting `getAllPosts()` to asynchronous in `scripts/build-rss.js` causes the existing build system to fail.
Learnt from: akshatnema
PR: asyncapi/website#3265
File: scripts/tools/tools-object.js:93-98
Timestamp: 2024-11-29T19:42:31.175Z
Learning: In `scripts/tools/tools-object.js`, when validation fails in the `convertTools` function, errors should be thrown or rejected instead of only logging them, to ensure proper error propagation and handling.
Learnt from: akshatnema
PR: asyncapi/website#3423
File: tests/index.test.js:2-7
Timestamp: 2025-01-18T08:44:43.614Z
Learning: In the AsyncAPI website project, JavaScript test files must include the .ts extension when importing TypeScript files (e.g., `require('../scripts/build-rss.ts')`). This is a project-specific requirement enforced by the linting rules and build setup, even though it differs from typical Node.js behavior.
Learnt from: anshgoyalevil
PR: asyncapi/website#3557
File: tests/fixtures/markdown/check-edit-links-data.js:3-11
Timestamp: 2025-01-19T04:51:41.255Z
Learning: In the AsyncAPI website repository, the test data in `tests/fixtures/markdown/check-edit-links-data.js` intentionally includes inconsistent paths (with and without 'docs' prefix) to verify the script's ability to normalize and handle ambiguous path structures.
Learnt from: akshatnema
PR: asyncapi/website#3378
File: scripts/markdown/check-markdown.js:1-1
Timestamp: 2024-11-25T18:41:29.632Z
Learning: When updating workflows for the AsyncAPI website repository, use `.github/workflows/if-nodejs-pr-testing.yml` to include environment variables and secrets for Node.js PR testing.
Learnt from: akshatnema
PR: asyncapi/website#3378
File: scripts/markdown/check-markdown.js:1-1
Timestamp: 2024-11-25T18:34:51.303Z
Learning: When reviewing `scripts/markdown/check-markdown.js`, optimizations should be addressed in separate issues and not included in the current pull request.
Learnt from: asyncapi-bot
PR: asyncapi/website#0
File: :0-0
Timestamp: 2025-02-18T12:07:42.211Z
Learning: The following PR commands are supported in the asyncapi/website repository:
- `/please-take-a-look` or `/ptal`: Requests attention from reviewers who haven't reviewed the PR
- `/ready-to-merge` or `/rtm`: Triggers automerge when all conditions are met
- `/do-not-merge` or `/dnm`: Blocks automerge even if all conditions are met
- `/autoupdate` or `/au`: Adds autoupdate label to keep PR in sync with target branch
- `/update` or `/u`: One-time update of PR with latest changes from target branch
Learnt from: JeelRajodiya
PR: asyncapi/website#3423
File: scripts/index.ts:29-31
Timestamp: 2025-03-16T15:55:44.257Z
Learning: In the AsyncAPI codebase, adding try-catch blocks around function calls is unnecessary when the only purpose is to add context to the error message, as errors will be thrown from the functions themselves with full stack traces that provide adequate context. Try-catch blocks should only be added when there's a need for graceful degradation, partial success behavior, or specific error handling logic.
Learnt from: JeelRajodiya
PR: asyncapi/website#3423
File: scripts/adopters/index.ts:9-11
Timestamp: 2024-12-31T09:25:52.573Z
Learning: The `writeJSON` function already includes error handling, so additional try/catch blocks around it may be redundant.
Learnt from: anshgoyalevil
PR: asyncapi/website#3557
File: .github/workflows/check-edit-links.yml:25-29
Timestamp: 2025-01-08T15:16:27.655Z
Learning: In GitHub workflows running scripts with process.exit statements for error handling (like check-editlinks.js in asyncapi/website), avoid adding error suppression (|| true) at the workflow level as it would mask the intended error reporting mechanism.
Learnt from: anshgoyalevil
PR: asyncapi/website#3301
File: scripts/markdown/check-markdown.js:0-0
Timestamp: 2024-10-18T17:24:45.053Z
Learning: In `scripts/markdown/check-markdown.js`, the script is not run in an asynchronous environment, so refactoring it to use `async/await` is unnecessary.
Learnt from: anshgoyalevil
PR: asyncapi/website#3950
File: scripts/utils/check-locales.ts:122-129
Timestamp: 2025-04-20T16:05:16.482Z
Learning: In the AsyncAPI website project, Next.js throws errors at runtime when locale files are missing, making additional validation for missing files unnecessary in the check-locales script.
npm/runners/build-newsroom-videos-runner.ts (18)
Learnt from: sagarkori143
PR: asyncapi/website#4192
File: npm/runners/build-pages-runner.ts:7-14
Timestamp: 2025-06-19T13:51:27.459Z
Learning: In the AsyncAPI website build system, individual runner functions use try-catch blocks that rethrow errors to maintain consistent error boundaries, while the top-level orchestrator in npm/index.ts provides meaningful error logging with task-specific context using logger.error(`Error building ${taskName}:`, error). This separation of concerns allows for clean error handling at different architectural layers.
Learnt from: sagarkori143
PR: asyncapi/website#0
File: :0-0
Timestamp: 2025-06-20T14:47:22.389Z
Learning: Runner files in the npm/runners directory should follow consistent error handling patterns, throwing new Error instances with descriptive messages rather than just rethrowing caught errors.
Learnt from: sagarkori143
PR: asyncapi/website#4192
File: npm/runners/build-newsroom-videos-runner.ts:8-15
Timestamp: 2025-06-19T13:49:29.796Z
Learning: In the AsyncAPI website modularization project, error handling is centralized in the top-level orchestrator function (npm/index.ts) with comprehensive logging and context. Individual runner functions in npm/runners/ are kept simple and let errors propagate naturally to the centralized handler, avoiding redundant try-catch blocks that only rethrow errors.
Learnt from: akshatnema
PR: asyncapi/website#3423
File: tests/index.test.js:2-7
Timestamp: 2025-01-18T08:44:43.614Z
Learning: In the AsyncAPI website project, JavaScript test files must include the .ts extension when importing TypeScript files (e.g., `require('../scripts/build-rss.ts')`). This is enforced by the project's configuration which uses `moduleResolution: "bundler"` in tsconfig.json and TypeScript-aware ESLint plugins. The .ts extensions are required even though the files are imported using CommonJS require statements.
Learnt from: akshatnema
PR: asyncapi/website#3101
File: scripts/build-rss.js:4-6
Timestamp: 2024-11-01T09:35:23.912Z
Learning: Converting `getAllPosts()` to asynchronous in `scripts/build-rss.js` causes the existing build system to fail.
Learnt from: akshatnema
PR: asyncapi/website#3423
File: tests/index.test.js:2-7
Timestamp: 2025-01-18T08:44:43.614Z
Learning: In the AsyncAPI website project, JavaScript test files must include the .ts extension when importing TypeScript files (e.g., `require('../scripts/build-rss.ts')`). This is a project-specific requirement enforced by the linting rules and build setup, even though it differs from typical Node.js behavior.
Learnt from: vishvamsinh28
PR: asyncapi/website#3284
File: scripts/build-post-list.js:49-53
Timestamp: 2024-11-10T18:16:22.273Z
Learning: In `scripts/build-post-list.js`, the `walkDirectories` function depends on sequential execution because it mutates `resultObj`. Parallelizing the loop with `Promise.all` causes tests to fail.
Learnt from: sagarkori143
PR: asyncapi/website#0
File: :0-0
Timestamp: 2025-06-20T14:47:22.389Z
Learning: The AsyncAPI website modularization PR uses Promise.allSettled for parallel execution of build tasks, which provides better performance and fault isolation compared to sequential execution.
Learnt from: anshgoyalevil
PR: asyncapi/website#3557
File: .github/workflows/check-edit-links.yml:25-29
Timestamp: 2025-01-08T15:16:27.655Z
Learning: In GitHub workflows running scripts with process.exit statements for error handling (like check-editlinks.js in asyncapi/website), avoid adding error suppression (|| true) at the workflow level as it would mask the intended error reporting mechanism.
Learnt from: anshgoyalevil
PR: asyncapi/website#3950
File: scripts/utils/check-locales.ts:122-129
Timestamp: 2025-04-20T16:05:16.482Z
Learning: In the AsyncAPI website project, Next.js throws errors at runtime when locale files are missing, making additional validation for missing files unnecessary in the check-locales script.
Learnt from: akshatnema
PR: asyncapi/website#3265
File: scripts/tools/tools-object.js:93-98
Timestamp: 2024-11-29T19:42:31.175Z
Learning: In `scripts/tools/tools-object.js`, when validation fails in the `convertTools` function, errors should be thrown or rejected instead of only logging them, to ensure proper error propagation and handling.
Learnt from: anshgoyalevil
PR: asyncapi/website#3557
File: tests/fixtures/markdown/check-edit-links-data.js:3-11
Timestamp: 2025-01-19T04:51:41.255Z
Learning: In the AsyncAPI website repository, the test data in `tests/fixtures/markdown/check-edit-links-data.js` intentionally includes inconsistent paths (with and without 'docs' prefix) to verify the script's ability to normalize and handle ambiguous path structures.
Learnt from: vishvamsinh28
PR: asyncapi/website#3284
File: scripts/build-post-list.js:28-46
Timestamp: 2024-11-10T18:16:35.551Z
Learning: In the `scripts/build-post-list.js` JavaScript file, tests rely on the `result` object being a global variable. Moving it inside the `buildPostList` function causes tests to fail; therefore, keep `result` as a global variable.
Learnt from: JeelRajodiya
PR: asyncapi/website#3423
File: scripts/index.ts:29-31
Timestamp: 2025-03-16T15:55:44.257Z
Learning: In the AsyncAPI codebase, adding try-catch blocks around function calls is unnecessary when the only purpose is to add context to the error message, as errors will be thrown from the functions themselves with full stack traces that provide adequate context. Try-catch blocks should only be added when there's a need for graceful degradation, partial success behavior, or specific error handling logic.
Learnt from: JeelRajodiya
PR: asyncapi/website#3423
File: scripts/adopters/index.ts:9-11
Timestamp: 2024-12-31T09:25:52.573Z
Learning: The `writeJSON` function already includes error handling, so additional try/catch blocks around it may be redundant.
Learnt from: anshgoyalevil
PR: asyncapi/website#3301
File: scripts/markdown/check-markdown.js:0-0
Timestamp: 2024-10-18T17:24:45.053Z
Learning: In `scripts/markdown/check-markdown.js`, the script is not run in an asynchronous environment, so refactoring it to use `async/await` is unnecessary.
Learnt from: akshatnema
PR: asyncapi/website#3265
File: tests/fixtures/toolsObjectData.js:51-52
Timestamp: 2024-10-09T17:35:36.557Z
Learning: When reviewing code in the 'asyncapi/website' repository, akshatnema prefers that I do not provide committable code suggestions.
Learnt from: akshatnema
PR: asyncapi/website#3265
File: scripts/tools/tools-object.js:49-51
Timestamp: 2024-11-01T11:40:51.617Z
Learning: In JavaScript code, prefer using a custom LogService with Winston for logging errors instead of `console.error` or ad-hoc logging.
npm/runners/build-meetings-runner.ts (11)
Learnt from: sagarkori143
PR: asyncapi/website#4192
File: npm/runners/build-pages-runner.ts:7-14
Timestamp: 2025-06-19T13:51:27.459Z
Learning: In the AsyncAPI website build system, individual runner functions use try-catch blocks that rethrow errors to maintain consistent error boundaries, while the top-level orchestrator in npm/index.ts provides meaningful error logging with task-specific context using logger.error(`Error building ${taskName}:`, error). This separation of concerns allows for clean error handling at different architectural layers.
Learnt from: sagarkori143
PR: asyncapi/website#4192
File: npm/runners/build-newsroom-videos-runner.ts:8-15
Timestamp: 2025-06-19T13:49:29.796Z
Learning: In the AsyncAPI website modularization project, error handling is centralized in the top-level orchestrator function (npm/index.ts) with comprehensive logging and context. Individual runner functions in npm/runners/ are kept simple and let errors propagate naturally to the centralized handler, avoiding redundant try-catch blocks that only rethrow errors.
Learnt from: sagarkori143
PR: asyncapi/website#0
File: :0-0
Timestamp: 2025-06-20T14:47:22.389Z
Learning: Runner files in the npm/runners directory should follow consistent error handling patterns, throwing new Error instances with descriptive messages rather than just rethrowing caught errors.
Learnt from: sagarkori143
PR: asyncapi/website#0
File: :0-0
Timestamp: 2025-06-20T14:47:22.389Z
Learning: The AsyncAPI website modularization PR uses Promise.allSettled for parallel execution of build tasks, which provides better performance and fault isolation compared to sequential execution.
Learnt from: vishvamsinh28
PR: asyncapi/website#3284
File: scripts/build-post-list.js:49-53
Timestamp: 2024-11-10T18:16:22.273Z
Learning: In `scripts/build-post-list.js`, the `walkDirectories` function depends on sequential execution because it mutates `resultObj`. Parallelizing the loop with `Promise.all` causes tests to fail.
Learnt from: akshatnema
PR: asyncapi/website#3101
File: scripts/build-rss.js:4-6
Timestamp: 2024-11-01T09:35:23.912Z
Learning: Converting `getAllPosts()` to asynchronous in `scripts/build-rss.js` causes the existing build system to fail.
Learnt from: akshatnema
PR: asyncapi/website#3265
File: scripts/tools/tools-object.js:93-98
Timestamp: 2024-11-29T19:42:31.175Z
Learning: In `scripts/tools/tools-object.js`, when validation fails in the `convertTools` function, errors should be thrown or rejected instead of only logging them, to ensure proper error propagation and handling.
Learnt from: anshgoyalevil
PR: asyncapi/website#3557
File: .github/workflows/check-edit-links.yml:25-29
Timestamp: 2025-01-08T15:16:27.655Z
Learning: In GitHub workflows running scripts with process.exit statements for error handling (like check-editlinks.js in asyncapi/website), avoid adding error suppression (|| true) at the workflow level as it would mask the intended error reporting mechanism.
Learnt from: anshgoyalevil
PR: asyncapi/website#3301
File: scripts/markdown/check-markdown.js:0-0
Timestamp: 2024-10-18T17:24:45.053Z
Learning: In `scripts/markdown/check-markdown.js`, the script is not run in an asynchronous environment, so refactoring it to use `async/await` is unnecessary.
Learnt from: anshgoyalevil
PR: asyncapi/website#3950
File: scripts/utils/check-locales.ts:122-129
Timestamp: 2025-04-20T16:05:16.482Z
Learning: In the AsyncAPI website project, Next.js throws errors at runtime when locale files are missing, making additional validation for missing files unnecessary in the check-locales script.
Learnt from: akshatnema
PR: asyncapi/website#3423
File: tests/index.test.js:2-7
Timestamp: 2025-01-18T08:44:43.614Z
Learning: In the AsyncAPI website project, JavaScript test files must include the .ts extension when importing TypeScript files (e.g., `require('../scripts/build-rss.ts')`). This is enforced by the project's configuration which uses `moduleResolution: "bundler"` in tsconfig.json and TypeScript-aware ESLint plugins. The .ts extensions are required even though the files are imported using CommonJS require statements.
npm/runners/build-pages-runner.ts (17)
Learnt from: sagarkori143
PR: asyncapi/website#4192
File: npm/runners/build-pages-runner.ts:7-14
Timestamp: 2025-06-19T13:51:27.459Z
Learning: In the AsyncAPI website build system, individual runner functions use try-catch blocks that rethrow errors to maintain consistent error boundaries, while the top-level orchestrator in npm/index.ts provides meaningful error logging with task-specific context using logger.error(`Error building ${taskName}:`, error). This separation of concerns allows for clean error handling at different architectural layers.
Learnt from: sagarkori143
PR: asyncapi/website#0
File: :0-0
Timestamp: 2025-06-20T14:47:22.389Z
Learning: Runner files in the npm/runners directory should follow consistent error handling patterns, throwing new Error instances with descriptive messages rather than just rethrowing caught errors.
Learnt from: sagarkori143
PR: asyncapi/website#4192
File: npm/runners/build-newsroom-videos-runner.ts:8-15
Timestamp: 2025-06-19T13:49:29.796Z
Learning: In the AsyncAPI website modularization project, error handling is centralized in the top-level orchestrator function (npm/index.ts) with comprehensive logging and context. Individual runner functions in npm/runners/ are kept simple and let errors propagate naturally to the centralized handler, avoiding redundant try-catch blocks that only rethrow errors.
Learnt from: vishvamsinh28
PR: asyncapi/website#3284
File: scripts/build-post-list.js:49-53
Timestamp: 2024-11-10T18:16:22.273Z
Learning: In `scripts/build-post-list.js`, the `walkDirectories` function depends on sequential execution because it mutates `resultObj`. Parallelizing the loop with `Promise.all` causes tests to fail.
Learnt from: akshatnema
PR: asyncapi/website#3101
File: scripts/build-rss.js:4-6
Timestamp: 2024-11-01T09:35:23.912Z
Learning: Converting `getAllPosts()` to asynchronous in `scripts/build-rss.js` causes the existing build system to fail.
Learnt from: anshgoyalevil
PR: asyncapi/website#3950
File: scripts/utils/check-locales.ts:122-129
Timestamp: 2025-04-20T16:05:16.482Z
Learning: In the AsyncAPI website project, Next.js throws errors at runtime when locale files are missing, making additional validation for missing files unnecessary in the check-locales script.
Learnt from: sagarkori143
PR: asyncapi/website#0
File: :0-0
Timestamp: 2025-06-20T14:47:22.389Z
Learning: The AsyncAPI website modularization PR uses Promise.allSettled for parallel execution of build tasks, which provides better performance and fault isolation compared to sequential execution.
Learnt from: akshatnema
PR: asyncapi/website#3423
File: tests/index.test.js:2-7
Timestamp: 2025-01-18T08:44:43.614Z
Learning: In the AsyncAPI website project, JavaScript test files must include the .ts extension when importing TypeScript files (e.g., `require('../scripts/build-rss.ts')`). This is a project-specific requirement enforced by the linting rules and build setup, even though it differs from typical Node.js behavior.
Learnt from: akshatnema
PR: asyncapi/website#3423
File: tests/index.test.js:2-7
Timestamp: 2025-01-18T08:44:43.614Z
Learning: In the AsyncAPI website project, JavaScript test files must include the .ts extension when importing TypeScript files (e.g., `require('../scripts/build-rss.ts')`). This is enforced by the project's configuration which uses `moduleResolution: "bundler"` in tsconfig.json and TypeScript-aware ESLint plugins. The .ts extensions are required even though the files are imported using CommonJS require statements.
Learnt from: anshgoyalevil
PR: asyncapi/website#3301
File: scripts/markdown/check-markdown.js:0-0
Timestamp: 2024-10-18T17:24:45.053Z
Learning: In `scripts/markdown/check-markdown.js`, the script is not run in an asynchronous environment, so refactoring it to use `async/await` is unnecessary.
Learnt from: JeelRajodiya
PR: asyncapi/website#3423
File: scripts/index.ts:29-31
Timestamp: 2025-03-16T15:55:44.257Z
Learning: In the AsyncAPI codebase, adding try-catch blocks around function calls is unnecessary when the only purpose is to add context to the error message, as errors will be thrown from the functions themselves with full stack traces that provide adequate context. Try-catch blocks should only be added when there's a need for graceful degradation, partial success behavior, or specific error handling logic.
Learnt from: anshgoyalevil
PR: asyncapi/website#3557
File: .github/workflows/check-edit-links.yml:25-29
Timestamp: 2025-01-08T15:16:27.655Z
Learning: In GitHub workflows running scripts with process.exit statements for error handling (like check-editlinks.js in asyncapi/website), avoid adding error suppression (|| true) at the workflow level as it would mask the intended error reporting mechanism.
Learnt from: JeelRajodiya
PR: asyncapi/website#3423
File: scripts/adopters/index.ts:9-11
Timestamp: 2024-12-31T09:25:52.573Z
Learning: The `writeJSON` function already includes error handling, so additional try/catch blocks around it may be redundant.
Learnt from: akshatnema
PR: asyncapi/website#3265
File: tests/fixtures/toolsObjectData.js:51-52
Timestamp: 2024-10-09T17:35:36.557Z
Learning: When reviewing code in the 'asyncapi/website' repository, akshatnema prefers that I do not provide committable code suggestions.
Learnt from: anshgoyalevil
PR: asyncapi/website#3557
File: scripts/markdown/check-editlinks.js:58-59
Timestamp: 2025-01-08T15:15:00.759Z
Learning: In the AsyncAPI codebase, batch processing operations (like in the Dashboard script and check-editlinks.js) follow a sequential pattern using await in loops, which is the preferred approach for maintaining consistency across the codebase.
Learnt from: akshatnema
PR: asyncapi/website#3265
File: scripts/tools/tools-object.js:93-98
Timestamp: 2024-11-29T19:42:31.175Z
Learning: In `scripts/tools/tools-object.js`, when validation fails in the `convertTools` function, errors should be thrown or rejected instead of only logging them, to ensure proper error propagation and handling.
Learnt from: vishvamsinh28
PR: asyncapi/website#3378
File: tests/markdown/check-markdown.test.js:133-138
Timestamp: 2024-11-29T17:36:09.783Z
Learning: When testing the 'main' function in 'check-markdown.test.js', it's acceptable to simply ensure it doesn't throw any errors, as the functions it calls are already tested elsewhere.
npm/runners/build-post-list-runner.ts (16)
Learnt from: vishvamsinh28
PR: asyncapi/website#3284
File: scripts/build-post-list.js:49-53
Timestamp: 2024-11-10T18:16:22.273Z
Learning: In `scripts/build-post-list.js`, the `walkDirectories` function depends on sequential execution because it mutates `resultObj`. Parallelizing the loop with `Promise.all` causes tests to fail.
Learnt from: akshatnema
PR: asyncapi/website#3101
File: scripts/build-rss.js:4-6
Timestamp: 2024-11-01T09:35:23.912Z
Learning: Converting `getAllPosts()` to asynchronous in `scripts/build-rss.js` causes the existing build system to fail.
Learnt from: sagarkori143
PR: asyncapi/website#4192
File: npm/runners/build-pages-runner.ts:7-14
Timestamp: 2025-06-19T13:51:27.459Z
Learning: In the AsyncAPI website build system, individual runner functions use try-catch blocks that rethrow errors to maintain consistent error boundaries, while the top-level orchestrator in npm/index.ts provides meaningful error logging with task-specific context using logger.error(`Error building ${taskName}:`, error). This separation of concerns allows for clean error handling at different architectural layers.
Learnt from: sagarkori143
PR: asyncapi/website#0
File: :0-0
Timestamp: 2025-06-20T14:47:22.389Z
Learning: Runner files in the npm/runners directory should follow consistent error handling patterns, throwing new Error instances with descriptive messages rather than just rethrowing caught errors.
Learnt from: sagarkori143
PR: asyncapi/website#4192
File: npm/runners/build-newsroom-videos-runner.ts:8-15
Timestamp: 2025-06-19T13:49:29.796Z
Learning: In the AsyncAPI website modularization project, error handling is centralized in the top-level orchestrator function (npm/index.ts) with comprehensive logging and context. Individual runner functions in npm/runners/ are kept simple and let errors propagate naturally to the centralized handler, avoiding redundant try-catch blocks that only rethrow errors.
Learnt from: vishvamsinh28
PR: asyncapi/website#3284
File: scripts/build-post-list.js:28-46
Timestamp: 2024-11-10T18:16:35.551Z
Learning: In the `scripts/build-post-list.js` JavaScript file, tests rely on the `result` object being a global variable. Moving it inside the `buildPostList` function causes tests to fail; therefore, keep `result` as a global variable.
Learnt from: sagarkori143
PR: asyncapi/website#0
File: :0-0
Timestamp: 2025-06-20T14:47:22.389Z
Learning: The AsyncAPI website modularization PR uses Promise.allSettled for parallel execution of build tasks, which provides better performance and fault isolation compared to sequential execution.
Learnt from: JeelRajodiya
PR: asyncapi/website#3423
File: scripts/index.ts:29-31
Timestamp: 2025-03-16T15:55:44.257Z
Learning: In the AsyncAPI codebase, adding try-catch blocks around function calls is unnecessary when the only purpose is to add context to the error message, as errors will be thrown from the functions themselves with full stack traces that provide adequate context. Try-catch blocks should only be added when there's a need for graceful degradation, partial success behavior, or specific error handling logic.
Learnt from: anshgoyalevil
PR: asyncapi/website#3950
File: scripts/utils/check-locales.ts:122-129
Timestamp: 2025-04-20T16:05:16.482Z
Learning: In the AsyncAPI website project, Next.js throws errors at runtime when locale files are missing, making additional validation for missing files unnecessary in the check-locales script.
Learnt from: anshgoyalevil
PR: asyncapi/website#3557
File: .github/workflows/check-edit-links.yml:25-29
Timestamp: 2025-01-08T15:16:27.655Z
Learning: In GitHub workflows running scripts with process.exit statements for error handling (like check-editlinks.js in asyncapi/website), avoid adding error suppression (|| true) at the workflow level as it would mask the intended error reporting mechanism.
Learnt from: JeelRajodiya
PR: asyncapi/website#3423
File: scripts/adopters/index.ts:9-11
Timestamp: 2024-12-31T09:25:52.573Z
Learning: The `writeJSON` function already includes error handling, so additional try/catch blocks around it may be redundant.
Learnt from: anshgoyalevil
PR: asyncapi/website#3557
File: tests/fixtures/markdown/check-edit-links-data.js:3-11
Timestamp: 2025-01-19T04:51:41.255Z
Learning: In the AsyncAPI website repository, the test data in `tests/fixtures/markdown/check-edit-links-data.js` intentionally includes inconsistent paths (with and without 'docs' prefix) to verify the script's ability to normalize and handle ambiguous path structures.
Learnt from: anshgoyalevil
PR: asyncapi/website#3301
File: scripts/markdown/check-markdown.js:0-0
Timestamp: 2024-10-18T17:24:45.053Z
Learning: In `scripts/markdown/check-markdown.js`, the script is not run in an asynchronous environment, so refactoring it to use `async/await` is unnecessary.
Learnt from: akshatnema
PR: asyncapi/website#3378
File: scripts/markdown/check-markdown.js:1-1
Timestamp: 2024-11-25T18:34:51.303Z
Learning: When reviewing `scripts/markdown/check-markdown.js`, optimizations should be addressed in separate issues and not included in the current pull request.
Learnt from: anshgoyalevil
PR: asyncapi/website#3301
File: scripts/markdown/check-markdown.js:0-0
Timestamp: 2024-10-18T17:28:41.718Z
Learning: In `scripts/markdown/check-markdown.js`, when processing large markdown files with recursion and loops, synchronous file reads (`fs.readFileSync`) are preferred over asynchronous reads.
Learnt from: akshatnema
PR: asyncapi/website#3136
File: scripts/tools/combine-tools.js:122-125
Timestamp: 2024-11-01T12:49:32.805Z
Learning: In `scripts/tools/combine-tools.js`, the existing URL parsing logic for `repoUrl` without additional error handling is acceptable and does not require changes.
npm/runners/build-rss-runner.ts (13)
Learnt from: akshatnema
PR: asyncapi/website#3101
File: scripts/build-rss.js:4-6
Timestamp: 2024-11-01T09:35:23.912Z
Learning: Converting `getAllPosts()` to asynchronous in `scripts/build-rss.js` causes the existing build system to fail.
Learnt from: sagarkori143
PR: asyncapi/website#4192
File: npm/runners/build-pages-runner.ts:7-14
Timestamp: 2025-06-19T13:51:27.459Z
Learning: In the AsyncAPI website build system, individual runner functions use try-catch blocks that rethrow errors to maintain consistent error boundaries, while the top-level orchestrator in npm/index.ts provides meaningful error logging with task-specific context using logger.error(`Error building ${taskName}:`, error). This separation of concerns allows for clean error handling at different architectural layers.
Learnt from: sagarkori143
PR: asyncapi/website#0
File: :0-0
Timestamp: 2025-06-20T14:47:22.389Z
Learning: Runner files in the npm/runners directory should follow consistent error handling patterns, throwing new Error instances with descriptive messages rather than just rethrowing caught errors.
Learnt from: akshatnema
PR: asyncapi/website#3101
File: tests/fixtures/rssData.js:1-57
Timestamp: 2024-11-01T13:32:15.472Z
Learning: In the `tests/fixtures/rssData.js` file of the `@asyncapi/website` project, tests for edge cases such as empty strings for title or excerpt, very long text content, international characters (UTF-8), or malformed URLs in `slug` or `cover` are not necessary because these cases will not occur.
Learnt from: akshatnema
PR: asyncapi/website#3101
File: tests/build-rss.test.js:25-27
Timestamp: 2024-11-01T09:55:20.531Z
Learning: In `tests/build-rss.test.js`, replacing `jest.resetModules()` with `jest.resetAllMocks()` in the `afterEach()` block causes errors. It is necessary to use `jest.resetModules()` to reset the module registry between tests in this file.
Learnt from: sagarkori143
PR: asyncapi/website#4192
File: npm/runners/build-newsroom-videos-runner.ts:8-15
Timestamp: 2025-06-19T13:49:29.796Z
Learning: In the AsyncAPI website modularization project, error handling is centralized in the top-level orchestrator function (npm/index.ts) with comprehensive logging and context. Individual runner functions in npm/runners/ are kept simple and let errors propagate naturally to the centralized handler, avoiding redundant try-catch blocks that only rethrow errors.
Learnt from: vishvamsinh28
PR: asyncapi/website#3284
File: scripts/build-post-list.js:49-53
Timestamp: 2024-11-10T18:16:22.273Z
Learning: In `scripts/build-post-list.js`, the `walkDirectories` function depends on sequential execution because it mutates `resultObj`. Parallelizing the loop with `Promise.all` causes tests to fail.
Learnt from: akshatnema
PR: asyncapi/website#3423
File: tests/index.test.js:2-7
Timestamp: 2025-01-18T08:44:43.614Z
Learning: In the AsyncAPI website project, JavaScript test files must include the .ts extension when importing TypeScript files (e.g., `require('../scripts/build-rss.ts')`). This is a project-specific requirement enforced by the linting rules and build setup, even though it differs from typical Node.js behavior.
Learnt from: akshatnema
PR: asyncapi/website#3423
File: tests/index.test.js:2-7
Timestamp: 2025-01-18T08:44:43.614Z
Learning: In the AsyncAPI website project, JavaScript test files must include the .ts extension when importing TypeScript files (e.g., `require('../scripts/build-rss.ts')`). This is enforced by the project's configuration which uses `moduleResolution: "bundler"` in tsconfig.json and TypeScript-aware ESLint plugins. The .ts extensions are required even though the files are imported using CommonJS require statements.
Learnt from: sagarkori143
PR: asyncapi/website#0
File: :0-0
Timestamp: 2025-06-20T14:47:22.389Z
Learning: The AsyncAPI website modularization PR uses Promise.allSettled for parallel execution of build tasks, which provides better performance and fault isolation compared to sequential execution.
Learnt from: JeelRajodiya
PR: asyncapi/website#3423
File: scripts/index.ts:29-31
Timestamp: 2025-03-16T15:55:44.257Z
Learning: In the AsyncAPI codebase, adding try-catch blocks around function calls is unnecessary when the only purpose is to add context to the error message, as errors will be thrown from the functions themselves with full stack traces that provide adequate context. Try-catch blocks should only be added when there's a need for graceful degradation, partial success behavior, or specific error handling logic.
Learnt from: anshgoyalevil
PR: asyncapi/website#3950
File: scripts/utils/check-locales.ts:122-129
Timestamp: 2025-04-20T16:05:16.482Z
Learning: In the AsyncAPI website project, Next.js throws errors at runtime when locale files are missing, making additional validation for missing files unnecessary in the check-locales script.
Learnt from: vishvamsinh28
PR: asyncapi/website#3284
File: scripts/build-post-list.js:28-46
Timestamp: 2024-11-10T18:16:35.551Z
Learning: In the `scripts/build-post-list.js` JavaScript file, tests rely on the `result` object being a global variable. Moving it inside the `buildPostList` function causes tests to fail; therefore, keep `result` as a global variable.
types/errors/CustomError.ts (2)
Learnt from: sagarkori143
PR: asyncapi/website#4192
File: npm/runners/build-pages-runner.ts:7-14
Timestamp: 2025-06-19T13:51:27.459Z
Learning: In the AsyncAPI website build system, individual runner functions use try-catch blocks that rethrow errors to maintain consistent error boundaries, while the top-level orchestrator in npm/index.ts provides meaningful error logging with task-specific context using logger.error(`Error building ${taskName}:`, error). This separation of concerns allows for clean error handling at different architectural layers.
Learnt from: JeelRajodiya
PR: asyncapi/website#3423
File: scripts/index.ts:29-31
Timestamp: 2025-03-16T15:55:44.257Z
Learning: In the AsyncAPI codebase, adding try-catch blocks around function calls is unnecessary when the only purpose is to add context to the error message, as errors will be thrown from the functions themselves with full stack traces that provide adequate context. Try-catch blocks should only be added when there's a need for graceful degradation, partial success behavior, or specific error handling logic.
🧬 Code Graph Analysis (2)
npm/runners/build-adopters-list-runner.ts (3)
scripts/adopters/index.ts (1)
buildAdoptersList(15-36)types/errors/RunnerError.ts (1)
RunnerError(22-22)scripts/helpers/logger.ts (1)
logger(87-87)
scripts/helpers/logger.ts (1)
types/errors/RunnerError.ts (1)
RunnerError(22-22)
🪛 Biome (1.9.4)
scripts/helpers/logger.ts
[error] 55-55: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
🪛 GitHub Check: codecov/patch
scripts/helpers/logger.ts
[warning] 23-24: scripts/helpers/logger.ts#L23-L24
Added lines #L23 - L24 were not covered by tests
[warning] 26-26: scripts/helpers/logger.ts#L26
Added line #L26 was not covered by tests
[warning] 44-44: scripts/helpers/logger.ts#L44
Added line #L44 was not covered by tests
[warning] 54-55: scripts/helpers/logger.ts#L54-L55
Added lines #L54 - L55 were not covered by tests
[warning] 73-74: scripts/helpers/logger.ts#L73-L74
Added lines #L73 - L74 were not covered by tests
[warning] 76-76: scripts/helpers/logger.ts#L76
Added line #L76 was not covered by tests
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Test NodeJS PR - windows-latest
- GitHub Check: Test NodeJS PR - macos-13
🔇 Additional comments (11)
npm/runners/build-dashboard-runner.ts (1)
25-84: Well-structured runner implementation!The error handling, conditional execution, and export pattern are implemented correctly. Good use of RunnerError with detailed context.
npm/runners/compose-blog-runner.ts (1)
26-88: Excellent implementation following established patterns!The runner correctly implements error handling with RunnerError, conditional execution, and proper exports.
npm/runners/build-finance-info-list-runner.ts (1)
1-136: Excellent implementation following established patterns.This runner exemplifies the high-quality modularization achieved in this PR. The implementation demonstrates:
- Robust error handling with
RunnerErrorand detailed context- Intelligent year detection logic for finance data
- Consistent logging and error propagation
- Proper TypeScript typing and documentation
- Follows the established architectural pattern perfectly
The file is production-ready and maintains consistency with other runners in the codebase.
npm/runners/build-newsroom-videos-runner.ts (1)
1-86: Solid implementation with excellent error handling.This runner demonstrates the refined error handling architecture achieved in this modularization effort:
- Proper
RunnerErrorusage with contextual metadata- Comprehensive logging with stack traces
- Clean separation between runner and script responsibilities
- Consistent with established patterns across all runners
The implementation is production-ready and exemplifies the quality improvements made in this PR.
npm/runners/build-tools-runner.ts (1)
1-105: Well-architected runner with integrated file handling.This runner showcases the comprehensive approach taken in the modularization:
- Integrates file reading logic directly in the runner layer
- Excellent error handling with
RunnerErrorand detailed context- Proper path resolution and configuration management
- Consistent logging and error propagation patterns
- Clean TypeScript implementation with proper typing
The file demonstrates how runners can take on additional responsibilities while maintaining architectural consistency.
npm/runners/build-meetings-runner.ts (1)
1-86: Clean and consistent implementation.This runner maintains the high standards established throughout the modularization effort:
- Proper
RunnerErrorusage with comprehensive context- Excellent error logging with structured metadata
- Consistent architectural patterns with other runners
- Clean TypeScript implementation
- Production-ready code quality
The implementation demonstrates the maturity and consistency achieved across all runner modules.
scripts/helpers/logger.ts (1)
21-82: Excellent error formatting infrastructure for the modularization effort.The enhanced logger provides sophisticated error handling with:
- Rich context extraction from
RunnerErrorinstances- Structured metadata formatting with field filtering
- Multi-line log output for better readability
- Proper integration with Winston's error handling
This logging infrastructure perfectly supports the new runner architecture and provides the detailed error context needed for debugging the modularized build system.
npm/runners/build-rss-runner.ts (2)
26-70: Excellent implementation of the runner pattern with comprehensive error handling.This runner demonstrates the mature modularization approach with:
- Configurable options: Flexible path configuration with sensible defaults
- Rich error context: Proper
RunnerErrorwrapping with operation, runner, script, and task details- Detailed logging: Full stack traces and contextual metadata for debugging
- Environment awareness: Conditional execution respecting test environments
The error handling provides significant value beyond simple rethrowing by adding structured context that integrates perfectly with the enhanced logger infrastructure.
72-85: Clean conditional execution and error propagation.The environment-based execution pattern with proper process exit handling ensures the runner integrates well with both development workflows and CI/CD pipelines.
npm/runners/build-pages-runner.ts (2)
25-72: Well-implemented runner with proper async handling and error context.This runner correctly implements the established pattern:
- Proper async usage: The
awaiton line 33 confirms the async declaration is appropriate- Comprehensive error handling: Errors are wrapped with detailed context before rethrowing
- Configuration flexibility: Options interface with sensible defaults
- Rich logging: Full error context including configuration and stack traces
The implementation addresses previous review concerns about async/sync mismatch and missing error rethrow. The error handling provides meaningful value by adding structured context for the top-level orchestrator.
74-87: Consistent conditional execution pattern.The environment-based execution with proper error handling follows the established pattern across all runners, ensuring reliable integration with both development and production workflows.
sambhavgupta0705
added
the
gsoc
Contributor
|
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-4192--asyncapi-website.netlify.app/ |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
npm/runners/build-tools-runner.ts (1)
32-36: Fix path resolution to use repository root.The config file paths are still resolved relative to the
npm/runners/directory instead of the repository root. This creates fragile paths that depend on the runner's location.- const automatedToolsPath = - options.automatedToolsPath || resolve(currentDirPath, '../../config', 'tools-automated.json'); - const manualToolsPath = options.manualToolsPath || resolve(currentDirPath, '../../config', 'tools-manual.json'); - const toolsPath = options.toolsPath || resolve(currentDirPath, '../../config', 'tools.json'); - const tagsPath = options.tagsPath || resolve(currentDirPath, '../../config', 'all-tags.json'); + const projectRoot = resolve(currentDirPath, '../../..'); + const automatedToolsPath = options.automatedToolsPath || resolve(projectRoot, 'config', 'tools-automated.json'); + const manualToolsPath = options.manualToolsPath || resolve(projectRoot, 'config', 'tools-manual.json'); + const toolsPath = options.toolsPath || resolve(projectRoot, 'config', 'tools.json'); + const tagsPath = options.tagsPath || resolve(projectRoot, 'config', 'all-tags.json');
🧹 Nitpick comments (3)
scripts/markdown/check-edit-links.ts (1)
61-61: UnnecessaryawaitbeforePromise.reject.The
awaitkeyword beforePromise.reject(...)is redundant sincePromise.rejectimmediately returns a rejected promise. The original version was more appropriate.- return await Promise.reject(new Error(`Error checking ${editLink}: ${error}`)); + return Promise.reject(new Error(`Error checking ${editLink}: ${error}`));npm/runners/build-adopters-list-runner.ts (1)
1-36: Consider adding export for consistency with other runners.Based on the established pattern in other runner modules, consider adding an export for the
runBuildAdoptersListfunction to maintain consistency and enable testing:+export async function runBuildAdoptersList(): Promise<void> { -async function runBuildAdoptersList(): Promise<void> {This would align with the modular architecture and testing patterns used throughout the codebase.
npm/runners/compose-blog-runner.ts (1)
59-71: Consider handling potential readline errors.The
promptUserfunction doesn't handle potential errors from the readline interface. While rare, readline can fail in certain environments.function promptUser(question: string): Promise<string> { const rl = readline.createInterface({ input: process.stdin, output: process.stdout }); - return new Promise((resolve) => { + return new Promise((resolve, reject) => { rl.question(question, (answer) => { rl.close(); resolve(answer.trim()); }); + + rl.on('error', (error) => { + rl.close(); + reject(error); + }); }); }
Contributor
Author
|
Hey @CodeRabbit can you please give a final but concise review of this PR. |
Contributor
|
Tip For best results, initiate chat on the files or code changes. @sagarkori143
✅ After that, this PR is production-ready and sets a new benchmark for the build system. Fantastic GSoC work—congrats! 🎉 🧠 Learnings used |
Contributor
|
@derberg @devilkiller-ag @sambhavgupta0705 @vishvamsinh28 @anshgoyalevil @Mayaleeeee Please take a look at this PR. Thanks! 👋 |
akshatnema
previously approved these changes
Aug 3, 2025
akshatnema
approved these changes
Aug 4, 2025
Member
|
/rtm |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request is related to to the Google summer of code 2025 project.
The PR includes the npm folder creation and successful migration of the following scripts:
Build Dashboard
Build newsroom videos
Build posts list
Build tools
Build case studies.
build finance
build adopters
build meetings
build pages
build rss feed
compose blogs
All of the above scripts are modularized such that they can be imported and run anywhere in the codebase. Still working on the PR. Opening it for intital reviews by coderabbit.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Style
Tests
Chores