-
Notifications
You must be signed in to change notification settings - Fork 3
Shared tsconfig.json & update options #29
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
This update enhances type safety and error handling by adopting optional chaining and non-null assertions, refactors the diff hunk parser for clearer control flow and stricter missing-hunk logging, and centralizes TypeScript compiler settings through a new base tsconfig with child configs now extending it. Walkthrough
Model: o4-mini | Prompt Tokens: 2598 | Completion Tokens: 780 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here is a friendly review of your code with AI support. Some suggestions come from predictions and might not fit every scenario. Feel free to pick the ideas that work best for you. You’re in charge of the decisions—AI is here simply to assist.
Model: o4-mini | Prompt Tokens: 3709 | Completion Tokens: 2266
| const hunkChangeMap = currentFile.chunks.flatMap(hunk => hunk.changes.map(change => ({ change, hunk }))) | ||
| let { change: startChange, hunk: startHunk } = hunkChangeMap[comment.start - 1] // eslint-disable-line prefer-const | ||
| let { change: endChange, hunk: endHunk } = hunkChangeMap[comment.end - 1] // eslint-disable-line prefer-const | ||
| const startEntry = hunkChangeMap[comment.start - 1] | ||
| const endEntry = hunkChangeMap[comment.end - 1] | ||
|
|
||
| if (!startHunk) { | ||
| core.warning(`Could not find hunk for comment on ${comment.path}, start ${comment.start}, end ${comment.end}, ${comment.comment}, skipping.`) | ||
| if (!startEntry || !endEntry) { | ||
| core.error(`Could not find hunk for comment on ${comment.path}, start ${comment.start}, end ${comment.end}, ${comment.comment}, skipping.`) | ||
| } else { | ||
| if (startHunk !== endHunk) endChange = startHunk.changes.at(-1)! | ||
| const { change: startChange, hunk: startHunk } = startEntry | ||
| let { change: endChange, hunk: endHunk } = endEntry // eslint-disable-line prefer-const |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The nested destructuring and multiple let/const statements make this block hard to follow. Extract the logic to find the start and end entries into a helper, and use early returns on missing data to reduce nesting. For instance:
function getHunkEntry(index: number) {
const entry = hunkChangeMap[index];
if (!entry) core.error(`Missing hunk entry at index ${index}`);
return entry;
}
const startEntry = getHunkEntry(comment.start - 1);
const endEntry = getHunkEntry(comment.end - 1);
if (!startEntry || !endEntry) return; // skipThis moves validation early, flattens the control flow, and makes the main logic more linear.
| const startSide = startChange.type !== "del" ? "RIGHT" : "LEFT" | ||
| const endSide = endChange.type !== "del" ? "RIGHT" : "LEFT" | ||
| if (!startHunk) { | ||
| core.warning(`Could not find hunk for comment on ${comment.path}, start ${comment.start}, end ${comment.end}, ${comment.comment}, skipping.`) | ||
| } else { | ||
| if (startHunk !== endHunk) endChange = startHunk.changes.at(-1)! | ||
|
|
||
| // get start line of the actual comment | ||
| let start: number | ||
| if (startChange.type === "normal") { | ||
| start = startChange.ln2 | ||
| } else if (startChange.type === "add" || startChange.type === "del") { | ||
| start = startChange.ln | ||
| } else throw new Error(`Unknown change type.`) | ||
| const startSide = startChange.type !== "del" ? "RIGHT" : "LEFT" | ||
| const endSide = endChange.type !== "del" ? "RIGHT" : "LEFT" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're repeating the same pattern to compute a line number based on change.type. Refactor into a utility:
function getLineNumber(change: Change): number {
switch (change.type) {
case 'normal': return change.ln2;
case 'add':
case 'del': return change.ln;
default: throw new Error(`Unknown change type: ${change.type}`);
}
}Then you can write:
const start = getLineNumber(startChange);
const end = getLineNumber(endChange);This removes repetition and centralizes error handling for unexpected change.type values.
|
Enabled TypeScript composite builds in both subprojects and linked them in the root config for faster incremental compilation. Also replaced a non-null assertion with optional chaining to prevent runtime errors when inputs lack defaults. Walkthrough
Model: o4-mini | Prompt Tokens: 536 | Completion Tokens: 479 | Diff Range: 1dea84d...d7fc237 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here is a friendly code review with AI support. These ideas are suggestions, not absolute truths, so adapt them to suit your project. You’re in charge, and AI is here to offer helpful guidance as you make final decisions.
Model: o4-mini | Prompt Tokens: 1684 | Completion Tokens: 2796 | Diff Range: 1dea84d...d7fc237
… add it to the main tsconfig
|
This patch centralizes TypeScript’s composite build setting by enabling it only in the root tsconfig.json, removes redundant composite flags in sub-project configs, and cleans up project references to simplify the overall build configuration without altering runtime behavior. Walkthrough
Model: o4-mini | Prompt Tokens: 470 | Completion Tokens: 1098 | Diff Range: d7fc237...246c361 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Enjoy this supportive AI-assisted code review offering ideas and suggestions without claiming certainty. Treat these points as options, not rules, and apply what fits your goals. Your judgment drives the final decisions—AI is here to lend a hand and inspire confidence in your process.
Model: o4-mini | Prompt Tokens: 1462 | Completion Tokens: 3474 | Diff Range: d7fc237...246c361
| { | ||
| "$schema": "https://json.schemastore.org/tsconfig", | ||
| "extends": "../tsconfig.json", | ||
| "compilerOptions": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You removed the $schema property, which provides IDE autocompletion and JSON validation. It’s helpful to keep it in your sub-project configs even when you extend a base. For example:
{
"$schema": "https://json.schemastore.org/tsconfig",
"extends": "../tsconfig.json",
"compilerOptions": {
"rootDir": "./src"
}
}|
This change integrates JSON Schema references into both TypeScript configuration files, enhancing editor support for schema validation and IntelliSense without altering build behavior. Walkthrough
Model: o4-mini | Prompt Tokens: 308 | Completion Tokens: 490 | Diff Range: 246c361...372bfa3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here’s a gentle AI-assisted look at your code with suggestions to consider, not strict rules. Feel free to apply what resonates and trust your expertise to guide the final decisions. You’re in control throughout, with AI here to support.
Model: o4-mini | Prompt Tokens: 1155 | Completion Tokens: 3181 | Diff Range: 246c361...372bfa3
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's a collaborative code review enhanced by AI assistance. These insights offer suggestions and patterns we've identified, though your expertise and judgment remain most important. Consider these recommendations as supportive guidance while you make the final decisions about your code. AI serves as a helpful tool in this process, but your experience and project knowledge are what truly guide the best choices.
Model: anthropic--claude-4-sonnet | Prompt Tokens: 4239 | Completion Tokens: 1306
* Update TypeScript configuration from leanix-sandbox:mob/main * Fix optional chaining in hunk reader and enforce non-null assertions in main * Update TypeScript configuration from leanix-sandbox:mob/main * Enforce non-null assertions for actionYaml inputs and regex match groups * Use shared tsconfig * Fix optional chaining for actionYaml input defaults in development environment * Add composite option to tsconfig files and update references * Remove composite option from pr-review and pr-summary tsconfig files; add it to the main tsconfig * Add property to pr-review and pr-summary tsconfig files
tsconfig.jsonfor both appsThis change implements stricter TypeScript configuration and improves error handling across the pr-review and pr-summary modules. The primary focus is on consolidating TypeScript settings into a shared configuration file and adding null-safety checks to prevent runtime errors when accessing potentially undefined object properties.
Walkthrough
Model: anthropic--claude-4-sonnet | Prompt Tokens: 3086 | Completion Tokens: 162