feat: new import command to import your existing Checkly resources to the CLI [sc-23506]
#1045
Conversation
WalkthroughThe diff adds comprehensive support for an import process to the CLI. A new test case verifies the CLI help output for the import command, and a corresponding topic entry is added in the package configuration. Several new command classes are introduced to handle import operations (apply, cancel, commit, plan). In addition, many new code generation modules are implemented across the constructs and source generation directories to support alert channels, checks, resource configurations, internal codegen utilities, and REST endpoints for managing import plans. Changes
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
🔇 Additional comments (6)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
sorccu
changed the title
import command to import your existing Checkly resources to the CLI [sc-23506]
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (38)
packages/cli/src/sourcegen/undefined.ts (1)
4-8: Implementation looks good.The
UndefinedValueclass correctly implements the rendering of the JavaScriptundefinedliteral. The class follows a clean pattern by extending the baseValueclass and implementing the requiredrendermethod.Consider adding a JSDoc comment to document the purpose of this class for better code maintainability.
packages/cli/src/sourcegen/null.ts (1)
4-8: Implementation looks good.The
NullValueclass correctly implements the rendering of the JavaScriptnullliteral. It follows the same clean pattern as the other value classes by extending the baseValueclass and implementing the requiredrendermethod.Consider adding a JSDoc comment to document the purpose of this class for better code maintainability and consistency with other value classes.
packages/cli/src/sourcegen/boolean.ts (1)
4-15: Implementation looks good.The
BooleanValueclass correctly implements the rendering of boolean values. It properly:
- Stores the boolean value as a property
- Calls super() in the constructor before accessing 'this'
- Uses toString() to convert the boolean to a string representation
This implementation is consistent with the value class pattern in this codebase.
Consider adding a JSDoc comment to document the purpose of this class for better code maintainability.
packages/cli/src/sourcegen/number.ts (1)
4-15: Implementation looks good with minor considerations.The
NumberValueclass correctly implements the rendering of number values, following the same pattern as the other value classes. It properly:
- Stores the number value as a property
- Calls super() in the constructor
- Uses toString() to convert the number to a string representation
Consider handling special number cases like
NaN,Infinity, or very large numbers that might produce unexpected string representations. You might want to add special handling for these cases if they need to be represented in a specific format in the generated code.Also, consider adding a JSDoc comment to document the purpose of this class for better code maintainability.
packages/cli/src/constructs/ref-codegen.ts (1)
4-13: Clean implementation of reference value generation.This function effectively generates the AST for a
Ref.from(...)expression, adding the necessary import and building the expression tree. This approach will work well for code generation use cases.Consider adding a brief documentation comment describing the expected output format of this function (e.g.,
Ref.from('ref-id')). This would improve clarity for maintainers unfamiliar with this part of the codebase.+/** + * Creates an expression representing a reference lookup using Ref.from('ref') + * @param program - The program to add imports to + * @param ref - The reference to generate a value for + * @returns A Value representing the Ref.from('ref') expression + */ export function valueForRef (program: Program, ref: Ref): Value {packages/cli/src/sourcegen/array.ts (1)
21-25: Consider handling trailing commas more explicitly.Currently, the render method adds a comma after each array element, including the last one. While trailing commas are valid in modern JavaScript/TypeScript, you might want to consider making this behavior configurable or more explicit for better clarity.
render (output: Output): void { if (this.value.length === 0) { output.append('[]') return } output.append('[') output.increaseIndent() - for (const value of this.value) { + for (let i = 0; i < this.value.length; i++) { + const value = this.value[i]; output.endLine() value.render(output) - output.append(',') + if (i < this.value.length - 1) { + output.append(',') + } } output.decreaseIndent() output.endLine() output.append(']') }packages/cli/src/sourcegen/object.ts (1)
26-33: Consider handling trailing commas more explicitly for object properties.Similar to the array implementation, the render method adds a comma after each property, including the last one. While trailing commas are valid in modern JavaScript/TypeScript, you might want to consider making this behavior configurable or more explicit for consistency and clarity.
render (output: Output): void { output.append('{') output.increaseIndent() - for (const { name, value } of this.value) { + for (let i = 0; i < this.value.length; i++) { + const { name, value } = this.value[i]; output.endLine() output.append(name) output.append(':') output.cosmeticWhitespace() value.render(output) - output.append(',') + if (i < this.value.length - 1) { + output.append(',') + } } output.decreaseIndent() output.endLine() output.append('}') }packages/cli/src/constructs/internal/codegen/codegen.ts (1)
11-13: Consider documenting the purpose of thepreparemethodThe
preparemethod is currently a no-op, but it might be intended for subclasses to override. Adding a comment explaining its purpose and when/how subclasses should use it would improve clarity.prepare (logicalId: string, resource: T, context: Context): void { - // No-op + // No-op by default. Subclasses can override this method to perform + // setup or validation before code generation. }packages/cli/src/sourcegen/string.ts (1)
19-22: Clarify the comment about using inspect()The comment notes that using
inspect()is "not ideal" but doesn't explain why or suggest a better approach. Consider adding more context to guide future improvements.- // Not ideal but for now inspect() works. + // Not ideal because inspect() includes Node.js-specific formatting that may change + // between versions, but it works for now. A more robust solution would be to implement + // proper string escaping for JavaScript/TypeScript syntax. return inspect(value)packages/cli/src/sourcegen/unknown.ts (1)
19-26: Consider adding a default case to handle all typesThe switch statement handles string, boolean, and number primitives, but doesn't explicitly handle other types like Symbol, BigInt, or functions. Adding a default case would make it more robust.
switch (typeof value) { case 'string': return new StringValue(value) case 'boolean': return new BooleanValue(value) case 'number': return new NumberValue(value) + default: + // For symbols, functions, bigints or other types we can't directly represent + // Convert to string representation as a fallback + return new StringValue(String(value)) }packages/cli/src/sourcegen/program.ts (2)
11-17: Simplify import method by removing redundant optional chainingSince you've already checked if the map has the key with
this.#imports.has(from), the optional chaining inthis.#imports.get(from)?.add(type)is unnecessary.import (type: string, from: string) { if (this.#imports.has(from)) { - this.#imports.get(from)?.add(type) + this.#imports.get(from)!.add(type) } else { this.#imports.set(from, new Set([type])) } }
23-45: Consider refactoring the import rendering logic for better readabilityThe complex rendering logic for imports could be extracted into a separate method to improve readability and maintainability.
render (output: Output): void { if (this.#imports.size > 0) { - for (const [pkg, types] of this.#imports.entries()) { - output.append('import') - output.cosmeticWhitespace() - output.append('{') - let first = true - for (const type of Array.from(types).sort()) { - if (!first) { - output.append(',') - } - output.cosmeticWhitespace() - first = false - output.append(type) - } - output.cosmeticWhitespace() - output.append('}') - output.significantWhitespace() - output.append('from') - output.significantWhitespace() - output.append(`'${pkg}'`) - } + this.#renderImports(output) } // ...rest of method... } + + #renderImports(output: Output): void { + for (const [pkg, types] of this.#imports.entries()) { + output.append('import') + output.cosmeticWhitespace() + output.append('{') + let first = true + for (const type of Array.from(types).sort()) { + if (!first) { + output.append(',') + } + output.cosmeticWhitespace() + first = false + output.append(type) + } + output.cosmeticWhitespace() + output.append('}') + output.significantWhitespace() + output.append('from') + output.significantWhitespace() + output.append(`'${pkg}'`) + } + }packages/cli/src/constructs/browser-check-codegen.ts (1)
25-32: TODO comment needs a more detailed plan.The TODO comment does not provide enough context about what exactly needs to be separated into a file and why. This makes it difficult for future developers to understand the intention.
Consider replacing the comment with a more detailed explanation:
- // TODO separate file + // TODO: Extract script content to a separate file in the repository + // and reference it via the entrypoint path for better maintainabilitypackages/cli/src/commands/import/commit.ts (2)
70-75: Misleading plan title for multiple plans.The plan title is hardcoded as "Plan #1" which is confusing when there are multiple plans being displayed in the list.
Update the title to include the plan's index or ID:
- const choices: prompts.Choice[] = plans.map(plan => ({ - title: `Plan #1 from ${new Date(plan.createdAt)}`, + const choices: prompts.Choice[] = plans.map((plan, index) => ({ + title: `Plan #${index + 1} from ${new Date(plan.createdAt)}`, value: plan.id, description: `ID: ${plan.id}`, }))
52-65: Error handling could be improved with more specific error messages.The current error handling just rethrows the error without providing context about what was being attempted.
Consider adding more context to the error:
try { await api.projects.commitImportPlan(plan.id) if (this.fancy) { ux.action.stop('✅ ') } } catch (err) { if (this.fancy) { ux.action.stop('❌') } - throw err + throw new Error(`Failed to commit plan ${plan.id}: ${err.message}`, { cause: err }) }packages/cli/src/commands/import/cancel.ts (2)
66-71: Misleading plan title for multiple plans.Similar to the commit command, the plan title is hardcoded as "Plan #1" which is confusing when there are multiple plans being displayed in the list.
Update the title to include the plan's index or ID:
- const choices: prompts.Choice[] = plans.map(plan => ({ - title: `Plan #1 from ${new Date(plan.createdAt)}`, + const choices: prompts.Choice[] = plans.map((plan, index) => ({ + title: `Plan #${index + 1} from ${new Date(plan.createdAt)}`, value: plan.id, description: `ID: ${plan.id}`, }))
47-63: Improved error handling would provide better context.When an error occurs during the cancellation of a plan, the current error handling just rethrows the error without context about which plan failed.
Consider modifying the error handling:
try { for (const plan of plans) { - await api.projects.cancelImportPlan(plan.id) - this.log(`${logSymbols.success} Canceled plan ${plan.id}`) + try { + await api.projects.cancelImportPlan(plan.id) + this.log(`${logSymbols.success} Canceled plan ${plan.id}`) + } catch (err) { + throw new Error(`Failed to cancel plan ${plan.id}: ${err.message}`, { cause: err }) + } } if (this.fancy) { ux.action.stop('✅ ') } } catch (err) { if (this.fancy) { ux.action.stop('❌') } throw err }packages/cli/src/constructs/dashboard-codegen.ts (1)
119-121: TODO comment about file separation needs more detail.The TODO comment is vague and doesn't provide specifics about when or why this refactoring should occur.
Consider adding more context:
- // TODO: Consider creating a separate file with the content - // and referring to it via entrypoint instead. + // TODO: For large CSS content, consider creating a separate .css file + // in the repository and referring to it via entrypoint instead to improve + // readability and enable proper syntax highlighting.packages/cli/src/constructs/telegram-alert-channel-codegen.ts (2)
12-17: Consider making the API key extraction more robustThe regex pattern assumes a specific URL format from Telegram's API. If their URL structure changes in the future, this extraction might fail.
-function apiKeyFromUrl (url: string): string | undefined { - const match = /https:\/\/api.telegram.org\/bot([^/]+)\/sendMessage/.exec(url) - if (match) { - return match[1] - } -} +function apiKeyFromUrl (url: string): string | undefined { + // Extract token between /bot and next slash from Telegram API URLs + const match = /api\.telegram\.org\/bot([^/]+)/.exec(url) + if (match) { + return match[1] + } +}
19-24: Consider adding validation for chat ID formatThe current implementation extracts any numeric string as a chat ID. Consider validating that it's a valid Telegram chat ID format.
function chatIdFromTemplate (template: string): string | undefined { const match = /chatId=(-?[0-9]+)/.exec(template) if (match) { + // Telegram chat IDs are numeric values and can be negative for groups return match[1] } }packages/cli/src/constructs/maintenance-window-codegen.ts (1)
16-64: Consider validating date strings or adding time-zone awareness.Since the code instantiates
Dateobjects based on unverified string inputs, it may be beneficial to inspect or validate these date strings. This could prevent unexpected runtime errors when the input is invalid or incorrectly formatted. Additionally, ensuring proper time-zone handling yields more predictable outcomes in different environments.Do you want me to propose a quick solution or add optional date parsing checks?
packages/cli/src/constructs/multi-step-check-codegen.ts (1)
16-41: Follow up on the TODO in the script handling.At line 25, there's a
// TODO separate filecomment. Separating script logic into its own module or file can promote cleaner organization and improved maintainability.Do you want me to open a tracking issue or propose a refactor for extracting the script logic?
packages/cli/src/sourcegen/decl.ts (1)
22-24: Consider expandingVariableDeclarationOptions.Right now, it's limited to a single
mutableflag. In the future, additional styling or scoping options (e.g., const enums) might be warranted for broader use.packages/cli/src/sourcegen/output.ts (1)
40-85: Consider adding validation to decreaseIndent method.While the Output class implementation is generally good, the decreaseIndent method doesn't check whether the level would become negative. Although this might not be an issue in the current implementation if calls to decreaseIndent are always balanced with increaseIndent, adding validation would make the code more robust.
Also, the methods significantWhitespace and cosmeticWhitespace currently do the same thing - they both append a single space. This suggests they might be intended to be differentiated in the future.
decreaseIndent () { + if (this.#level > 0) { this.#level -= 1 + } else { + console.warn('Attempted to decrease indent below 0') + } }packages/cli/src/commands/import/apply.ts (1)
22-60: Consider adding validation for configFilename.The run method implementation is good, but it doesn't validate whether configFilename is defined. If a user doesn't provide the --config flag, this could lead to errors in the subsequent operations.
Also, there's no handling for the case where no unapplied plans are found. Consider providing a clear message to the user in this scenario.
async run (): Promise<void> { const { flags } = await this.parse(ImportApplyCommand) const { config: configFilename, } = flags + if (!configFilename) { + this.error('No configuration file specified. Use --config flag to specify a configuration file.') + return + } const { configDirectory, configFilenames } = splitConfigFilePath(configFilename) // Rest of the method... const { data: unappliedPlans } = await api.projects.findImportPlans(logicalId, { onlyUnapplied: true, }) + if (unappliedPlans.length === 0) { + this.log('No unapplied import plans found.') + return + } const plan = await this.#selectPlan(unappliedPlans) // Rest of the method... }packages/cli/src/constructs/api-check-codegen.ts (2)
110-123: Handling setup scripts from string content or file paths.
Defining separate objects forlocalSetupScriptandsetupScriptPathis fine. However, if both are provided, we’ll end up with twosetupScriptobjects in the final output. It might be beneficial to enforce a mutual exclusion or define a preference.Would you like help adding a fallback or merging these properties to avoid potential conflicts?
125-138: Symmetrical teardown script logic.
Mirroring the approach from the setup script is consistent. Keep in mind the same mutual-exclusion considerations if bothlocalTearDownScriptandtearDownScriptPathare set.packages/cli/src/constructs/check-codegen.ts (1)
164-181: Lint warnings about returning a value from a void function.
Theswitchstatements immediately return calls to other.gencode()methods. If these are typed to returnvoid, removingreturnwould align better with certain lint rules:- return this.browserCheckCodegen.gencode(logicalId, resource as BrowserCheckResource, context) + this.browserCheckCodegen.gencode(logicalId, resource as BrowserCheckResource, context)And similarly for lines 171, 173, 175, and 177.
🧰 Tools
🪛 Biome (1.9.4)
[error] 169-169: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
[error] 171-171: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
[error] 173-173: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
[error] 175-175: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
[error] 177-177: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
packages/cli/src/commands/import/plan.ts (3)
52-82: Handle additional HTTP error codes gracefully.
Currently, only 404 status codes have a custom error message. Consider adding user-friendly messages for other common errors such as 401 or 403 to improve the developer experience.
86-129: Allow user-specified output paths for generated code.
The generated file path is currently hardcoded. Offering a user-configurable file/directory path can enhance flexibility.Would you like help implementing a solution that accepts an output path?
130-223: Improve partial cancellation flow for existing plans.
When canceling multiple plans, an error in one plan halts the entire cancellation loop. Consider allowing partial cancellation or a rollback mechanism for unprocessed plans.packages/cli/src/constructs/tcp-check-codegen.ts (1)
7-12: Validate response time fields.
Consider enforcing a logical constraint (e.g.,degradedResponseTime <= maxResponseTime) to avoid confusion or invalid configurations.packages/cli/src/constructs/incidentio-alert-channel-codegen.ts (1)
13-25: API key extraction could be improved with early returnsThe
apiKeyFromHeadersfunction extracts the API key from the Authorization header, but the implementation could be simplified using early returns for better readability.function apiKeyFromHeaders (headers: HttpHeader[]): string | undefined { for (const header of headers) { - if (header.key.toLocaleLowerCase() !== 'authorization') { - continue - } - - if (!header.value.startsWith('Bearer ')) { - continue - } - - return header.value.replace('Bearer ', '') + if (header.key.toLocaleLowerCase() === 'authorization' && header.value.startsWith('Bearer ')) { + return header.value.replace('Bearer ', '') + } } + return undefined }packages/cli/src/constructs/construct-codegen.ts (1)
25-29:Resourceinterface is minimal yet effective.
payload: anydoes reduce type safety. In future, consider parameterizing the interface or introducing type refinements per resource type.-interface Resource { - type: ResourceType - logicalId: string - payload: any -} +interface Resource<TPayload = unknown> { + type: ResourceType + logicalId: string + payload: TPayload +}packages/cli/src/rest/projects.ts (1)
85-92: Minor improvement for filtering logic
The usage of optional parameters foronlyUnappliedandonlyUncommittedis correct. Consider whether the server should handle contradictory filters or if you want to guard them here.packages/cli/src/sourcegen/expr.ts (1)
42-57: Consider bracket notation for invalid identifiers
The comment at line 55 hints at handling property names that require bracket syntax. Consider adding logic to detect such cases and use bracket notation when needed.packages/cli/src/constructs/check-group-codegen.ts (1)
43-227: Consider better error handling for context lookups
The function currently swallows errors when looking up private locations or alert channels (lines 72–77, 110–115). Logging or graceful fallback might be preferable. Also consider splitting this function into smaller parts for easier maintainability.packages/cli/src/constructs/internal/codegen/context.ts (1)
3-4: Consider adding a more descriptive error message.
Throwing a bareMissingContextVariableMappingErrorcan make debugging harder. Including the requested ID or a custom message aids in tracing the missing mapping.-export class MissingContextVariableMappingError extends Error {} +export class MissingContextVariableMappingError extends Error { + constructor(message?: string) { + super(message || 'Context variable mapping not found.') + this.name = 'MissingContextVariableMappingError' + } +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (65)
packages/cli/e2e/__tests__/help.spec.ts(2 hunks)packages/cli/package.json(1 hunks)packages/cli/src/commands/import/apply.ts(1 hunks)packages/cli/src/commands/import/cancel.ts(1 hunks)packages/cli/src/commands/import/commit.ts(1 hunks)packages/cli/src/commands/import/plan.ts(1 hunks)packages/cli/src/constructs/alert-channel-codegen.ts(1 hunks)packages/cli/src/constructs/alert-channel-subscription-codegen.ts(1 hunks)packages/cli/src/constructs/alert-escalation-policy-codegen.ts(1 hunks)packages/cli/src/constructs/api-check-codegen.ts(1 hunks)packages/cli/src/constructs/browser-check-codegen.ts(1 hunks)packages/cli/src/constructs/check-codegen.ts(1 hunks)packages/cli/src/constructs/check-group-codegen.ts(1 hunks)packages/cli/src/constructs/check-group.ts(1 hunks)packages/cli/src/constructs/construct-codegen.ts(1 hunks)packages/cli/src/constructs/dashboard-codegen.ts(1 hunks)packages/cli/src/constructs/email-alert-channel-codegen.ts(1 hunks)packages/cli/src/constructs/frequency-codegen.ts(1 hunks)packages/cli/src/constructs/heartbeat-check-codegen.ts(1 hunks)packages/cli/src/constructs/incidentio-alert-channel-codegen.ts(1 hunks)packages/cli/src/constructs/internal/assertion-codegen.ts(1 hunks)packages/cli/src/constructs/internal/codegen/codegen.ts(1 hunks)packages/cli/src/constructs/internal/codegen/context.ts(1 hunks)packages/cli/src/constructs/internal/codegen/index.ts(1 hunks)packages/cli/src/constructs/key-value-pair-codegen.ts(1 hunks)packages/cli/src/constructs/maintenance-window-codegen.ts(1 hunks)packages/cli/src/constructs/msteams-alert-channel-codegen.ts(1 hunks)packages/cli/src/constructs/multi-step-check-codegen.ts(1 hunks)packages/cli/src/constructs/opsgenie-alert-channel-codegen.ts(1 hunks)packages/cli/src/constructs/pagerduty-alert-channel-codegen.ts(1 hunks)packages/cli/src/constructs/phone-call-alert-channel-codegen.ts(1 hunks)packages/cli/src/constructs/playwright-config-codegen.ts(1 hunks)packages/cli/src/constructs/playwright-config.ts(0 hunks)packages/cli/src/constructs/private-location-check-assignment-codegen.ts(1 hunks)packages/cli/src/constructs/private-location-codegen.ts(1 hunks)packages/cli/src/constructs/private-location-group-assignment-codegen.ts(1 hunks)packages/cli/src/constructs/ref-codegen.ts(1 hunks)packages/cli/src/constructs/retry-strategy-codegen.ts(1 hunks)packages/cli/src/constructs/slack-alert-channel-codegen.ts(1 hunks)packages/cli/src/constructs/sms-alert-channel-codegen.ts(1 hunks)packages/cli/src/constructs/tcp-check-codegen.ts(1 hunks)packages/cli/src/constructs/telegram-alert-channel-codegen.ts(1 hunks)packages/cli/src/constructs/webhook-alert-channel-codegen.ts(1 hunks)packages/cli/src/rest/projects.ts(3 hunks)packages/cli/src/sourcegen/args.ts(1 hunks)packages/cli/src/sourcegen/argsbuilder.ts(1 hunks)packages/cli/src/sourcegen/array.ts(1 hunks)packages/cli/src/sourcegen/arraybuilder.ts(1 hunks)packages/cli/src/sourcegen/boolean.ts(1 hunks)packages/cli/src/sourcegen/decl.ts(1 hunks)packages/cli/src/sourcegen/declbuilder.ts(1 hunks)packages/cli/src/sourcegen/expr.ts(1 hunks)packages/cli/src/sourcegen/exprbuilder.ts(1 hunks)packages/cli/src/sourcegen/identifier.ts(1 hunks)packages/cli/src/sourcegen/index.ts(1 hunks)packages/cli/src/sourcegen/null.ts(1 hunks)packages/cli/src/sourcegen/number.ts(1 hunks)packages/cli/src/sourcegen/object.ts(1 hunks)packages/cli/src/sourcegen/objectbuilder.ts(1 hunks)packages/cli/src/sourcegen/output.ts(1 hunks)packages/cli/src/sourcegen/program.ts(1 hunks)packages/cli/src/sourcegen/string.ts(1 hunks)packages/cli/src/sourcegen/undefined.ts(1 hunks)packages/cli/src/sourcegen/unknown.ts(1 hunks)packages/cli/src/sourcegen/value.ts(1 hunks)
💤 Files with no reviewable changes (1)
- packages/cli/src/constructs/playwright-config.ts
🧰 Additional context used
🧬 Code Definitions (42)
packages/cli/src/sourcegen/undefined.ts (2)
packages/cli/src/sourcegen/index.ts (3)
UndefinedValue(31-31)Value(33-33)Output(28-28)packages/cli/src/sourcegen/output.ts (1)
Output(40-85)
packages/cli/src/sourcegen/null.ts (2)
packages/cli/src/sourcegen/index.ts (3)
NullValue(24-24)Value(33-33)Output(28-28)packages/cli/src/sourcegen/output.ts (1)
Output(40-85)
packages/cli/src/sourcegen/boolean.ts (2)
packages/cli/src/sourcegen/index.ts (3)
BooleanValue(5-5)Value(33-33)Output(28-28)packages/cli/src/sourcegen/output.ts (1)
Output(40-85)
packages/cli/e2e/__tests__/help.spec.ts (1)
packages/cli/e2e/run-checkly.ts (1)
runChecklyCli(7-77)
packages/cli/src/sourcegen/value.ts (1)
packages/cli/src/sourcegen/output.ts (1)
Output(40-85)
packages/cli/src/sourcegen/array.ts (2)
packages/cli/src/sourcegen/index.ts (3)
ArrayValue(3-3)Value(33-33)Output(28-28)packages/cli/src/sourcegen/output.ts (1)
Output(40-85)
packages/cli/src/sourcegen/object.ts (4)
packages/cli/src/sourcegen/index.ts (3)
Value(33-33)ObjectValue(26-26)Output(28-28)packages/cli/src/sourcegen/arraybuilder.ts (1)
value(62-65)packages/cli/src/sourcegen/objectbuilder.ts (1)
value(57-60)packages/cli/src/sourcegen/output.ts (1)
Output(40-85)
packages/cli/src/sourcegen/unknown.ts (7)
packages/cli/src/sourcegen/arraybuilder.ts (4)
value(62-65)array(12-16)array(50-52)object(54-56)packages/cli/src/sourcegen/objectbuilder.ts (4)
value(57-60)array(45-47)object(12-16)object(49-51)packages/cli/src/sourcegen/null.ts (1)
NullValue(4-8)packages/cli/src/sourcegen/undefined.ts (1)
UndefinedValue(4-8)packages/cli/src/sourcegen/string.ts (1)
StringValue(6-17)packages/cli/src/sourcegen/boolean.ts (1)
BooleanValue(4-15)packages/cli/src/sourcegen/number.ts (1)
NumberValue(4-15)
packages/cli/src/constructs/email-alert-channel-codegen.ts (3)
packages/cli/src/constructs/alert-channel-codegen.ts (2)
AlertChannelResource(21-29)buildAlertChannelProps(31-51)packages/cli/src/constructs/internal/codegen/index.ts (2)
Codegen(1-1)Context(2-2)packages/cli/src/constructs/internal/codegen/context.ts (1)
Context(5-121)
packages/cli/src/constructs/multi-step-check-codegen.ts (4)
packages/cli/src/constructs/check-codegen.ts (2)
CheckResource(15-33)buildCheckProps(35-144)packages/cli/src/constructs/playwright-config-codegen.ts (2)
PlaywrightConfigResource(4-4)valueForPlaywrightConfig(6-8)packages/cli/src/constructs/internal/codegen/index.ts (2)
Codegen(1-1)Context(2-2)packages/cli/src/constructs/internal/codegen/context.ts (1)
Context(5-121)
packages/cli/src/constructs/private-location-check-assignment-codegen.ts (2)
packages/cli/src/constructs/internal/codegen/index.ts (2)
Codegen(1-1)Context(2-2)packages/cli/src/constructs/internal/codegen/context.ts (1)
Context(5-121)
packages/cli/src/constructs/telegram-alert-channel-codegen.ts (4)
packages/cli/src/constructs/webhook-alert-channel-codegen.ts (2)
WebhookAlertChannelResource(28-31)WebhookAlertChannelResourceConfig(17-26)packages/cli/src/constructs/internal/codegen/index.ts (2)
Codegen(1-1)Context(2-2)packages/cli/src/constructs/internal/codegen/context.ts (1)
Context(5-121)packages/cli/src/constructs/alert-channel-codegen.ts (1)
buildAlertChannelProps(31-51)
packages/cli/src/sourcegen/output.ts (3)
packages/cli/src/sourcegen/arraybuilder.ts (1)
value(62-65)packages/cli/src/sourcegen/objectbuilder.ts (1)
value(57-60)packages/cli/src/sourcegen/index.ts (1)
Output(28-28)
packages/cli/src/constructs/private-location-group-assignment-codegen.ts (2)
packages/cli/src/constructs/internal/codegen/index.ts (2)
Codegen(1-1)Context(2-2)packages/cli/src/constructs/internal/codegen/context.ts (1)
Context(5-121)
packages/cli/src/constructs/browser-check-codegen.ts (4)
packages/cli/src/constructs/check-codegen.ts (2)
CheckResource(15-33)buildCheckProps(35-144)packages/cli/src/constructs/playwright-config-codegen.ts (2)
PlaywrightConfigResource(4-4)valueForPlaywrightConfig(6-8)packages/cli/src/constructs/internal/codegen/index.ts (2)
Codegen(1-1)Context(2-2)packages/cli/src/constructs/internal/codegen/context.ts (1)
Context(5-121)
packages/cli/src/sourcegen/identifier.ts (4)
packages/cli/src/sourcegen/index.ts (4)
IdentifierValue(23-23)Value(33-33)Output(28-28)ident(23-23)packages/cli/src/sourcegen/arraybuilder.ts (2)
value(62-65)ident(46-48)packages/cli/src/sourcegen/objectbuilder.ts (2)
value(57-60)ident(41-43)packages/cli/src/sourcegen/output.ts (1)
Output(40-85)
packages/cli/src/constructs/api-check-codegen.ts (5)
packages/cli/src/constructs/check-codegen.ts (2)
CheckResource(15-33)buildCheckProps(35-144)packages/cli/src/constructs/internal/assertion-codegen.ts (2)
valueForNumericAssertion(4-43)valueForGeneralAssertion(45-146)packages/cli/src/constructs/internal/codegen/index.ts (2)
Codegen(1-1)Context(2-2)packages/cli/src/constructs/internal/codegen/context.ts (1)
Context(5-121)packages/cli/src/constructs/key-value-pair-codegen.ts (1)
valueForKeyValuePair(4-17)
packages/cli/src/sourcegen/string.ts (4)
packages/cli/src/sourcegen/index.ts (3)
StringValue(30-30)Value(33-33)Output(28-28)packages/cli/src/sourcegen/arraybuilder.ts (1)
value(62-65)packages/cli/src/sourcegen/objectbuilder.ts (1)
value(57-60)packages/cli/src/sourcegen/output.ts (1)
Output(40-85)
packages/cli/src/constructs/tcp-check-codegen.ts (4)
packages/cli/src/constructs/check-codegen.ts (2)
CheckResource(15-33)buildCheckProps(35-144)packages/cli/src/constructs/internal/assertion-codegen.ts (2)
valueForGeneralAssertion(45-146)valueForNumericAssertion(4-43)packages/cli/src/constructs/internal/codegen/index.ts (2)
Codegen(1-1)Context(2-2)packages/cli/src/constructs/internal/codegen/context.ts (1)
Context(5-121)
packages/cli/src/constructs/alert-channel-subscription-codegen.ts (1)
packages/cli/src/constructs/internal/codegen/context.ts (1)
Context(5-121)
packages/cli/src/constructs/dashboard-codegen.ts (2)
packages/cli/src/constructs/internal/codegen/index.ts (2)
Codegen(1-1)Context(2-2)packages/cli/src/constructs/internal/codegen/context.ts (1)
Context(5-121)
packages/cli/src/constructs/pagerduty-alert-channel-codegen.ts (3)
packages/cli/src/constructs/alert-channel-codegen.ts (2)
AlertChannelResource(21-29)buildAlertChannelProps(31-51)packages/cli/src/constructs/internal/codegen/index.ts (2)
Codegen(1-1)Context(2-2)packages/cli/src/constructs/internal/codegen/context.ts (1)
Context(5-121)
packages/cli/src/constructs/opsgenie-alert-channel-codegen.ts (3)
packages/cli/src/constructs/alert-channel-codegen.ts (2)
AlertChannelResource(21-29)buildAlertChannelProps(31-51)packages/cli/src/constructs/internal/codegen/index.ts (2)
Codegen(1-1)Context(2-2)packages/cli/src/constructs/internal/codegen/context.ts (1)
Context(5-121)
packages/cli/src/sourcegen/declbuilder.ts (2)
packages/cli/src/sourcegen/index.ts (6)
decl(11-11)IdentifierValue(23-23)DeclarationBuilder(12-12)Value(33-33)VariableDeclaration(7-7)ExportDeclaration(8-8)packages/cli/src/sourcegen/decl.ts (4)
IdentifierDeclaration(9-20)VariableDeclarationOptions(22-24)VariableDeclaration(26-52)ExportDeclaration(54-66)
packages/cli/src/constructs/construct-codegen.ts (12)
packages/cli/src/constructs/internal/codegen/index.ts (2)
Codegen(1-1)Context(2-2)packages/cli/src/constructs/alert-channel-codegen.ts (1)
AlertChannelCodegen(53-101)packages/cli/src/constructs/alert-channel-subscription-codegen.ts (1)
AlertChannelSubscriptionCodegen(9-24)packages/cli/src/constructs/check-codegen.ts (1)
CheckCodegen(146-182)packages/cli/src/constructs/check-group-codegen.ts (1)
CheckGroupCodegen(231-252)packages/cli/src/constructs/dashboard-codegen.ts (1)
DashboardCodegen(31-140)packages/cli/src/constructs/maintenance-window-codegen.ts (1)
MaintenanceWindowCodegen(16-64)packages/cli/src/constructs/private-location-codegen.ts (1)
PrivateLocationCodegen(14-44)packages/cli/src/constructs/private-location-check-assignment-codegen.ts (1)
PrivateLocationCheckAssignmentCodegen(8-17)packages/cli/src/constructs/private-location-group-assignment-codegen.ts (1)
PrivateLocationGroupAssignmentCodegen(8-17)packages/cli/src/sourcegen/program.ts (1)
Program(7-53)packages/cli/src/constructs/internal/codegen/context.ts (1)
Context(5-121)
packages/cli/src/constructs/maintenance-window-codegen.ts (2)
packages/cli/src/constructs/internal/codegen/index.ts (2)
Codegen(1-1)Context(2-2)packages/cli/src/constructs/internal/codegen/context.ts (1)
Context(5-121)
packages/cli/src/constructs/slack-alert-channel-codegen.ts (2)
packages/cli/src/constructs/alert-channel-codegen.ts (2)
AlertChannelResource(21-29)buildAlertChannelProps(31-51)packages/cli/src/constructs/internal/codegen/context.ts (1)
Context(5-121)
packages/cli/src/constructs/check-codegen.ts (6)
packages/cli/src/constructs/frequency-codegen.ts (2)
FrequencyResource(4-4)valueForFrequency(6-54)packages/cli/src/constructs/alert-escalation-policy-codegen.ts (2)
AlertEscalationResource(4-4)valueForAlertEscalation(6-65)packages/cli/src/constructs/retry-strategy-codegen.ts (2)
RetryStrategyResource(4-4)valueForRetryStrategy(6-77)packages/cli/src/constructs/internal/codegen/index.ts (2)
Context(2-2)Codegen(1-1)packages/cli/src/constructs/key-value-pair-codegen.ts (1)
valueForKeyValuePair(4-17)packages/cli/src/constructs/api-check-codegen.ts (1)
ApiCheckResource(8-19)
packages/cli/src/sourcegen/decl.ts (3)
packages/cli/src/sourcegen/output.ts (1)
Output(40-85)packages/cli/src/sourcegen/index.ts (6)
Output(28-28)IdentifierValue(23-23)VariableDeclaration(7-7)Value(33-33)ExportDeclaration(8-8)decl(11-11)packages/cli/src/sourcegen/declbuilder.ts (1)
decl(5-9)
packages/cli/src/constructs/incidentio-alert-channel-codegen.ts (4)
packages/cli/src/constructs/webhook-alert-channel-codegen.ts (2)
WebhookAlertChannelResource(28-31)WebhookAlertChannelResourceConfig(17-26)packages/cli/src/constructs/internal/codegen/index.ts (2)
Codegen(1-1)Context(2-2)packages/cli/src/constructs/internal/codegen/context.ts (1)
Context(5-121)packages/cli/src/constructs/alert-channel-codegen.ts (1)
buildAlertChannelProps(31-51)
packages/cli/src/constructs/private-location-codegen.ts (2)
packages/cli/src/constructs/internal/codegen/index.ts (2)
Codegen(1-1)Context(2-2)packages/cli/src/constructs/internal/codegen/context.ts (1)
Context(5-121)
packages/cli/src/constructs/msteams-alert-channel-codegen.ts (4)
packages/cli/src/constructs/webhook-alert-channel-codegen.ts (2)
WebhookAlertChannelResource(28-31)WebhookAlertChannelResourceConfig(17-26)packages/cli/src/constructs/internal/codegen/index.ts (2)
Codegen(1-1)Context(2-2)packages/cli/src/constructs/internal/codegen/context.ts (1)
Context(5-121)packages/cli/src/constructs/alert-channel-codegen.ts (1)
buildAlertChannelProps(31-51)
packages/cli/src/constructs/sms-alert-channel-codegen.ts (2)
packages/cli/src/constructs/alert-channel-codegen.ts (2)
AlertChannelResource(21-29)buildAlertChannelProps(31-51)packages/cli/src/constructs/internal/codegen/context.ts (1)
Context(5-121)
packages/cli/src/constructs/phone-call-alert-channel-codegen.ts (3)
packages/cli/src/constructs/alert-channel-codegen.ts (2)
AlertChannelResource(21-29)buildAlertChannelProps(31-51)packages/cli/src/constructs/internal/codegen/index.ts (2)
Codegen(1-1)Context(2-2)packages/cli/src/constructs/internal/codegen/context.ts (1)
Context(5-121)
packages/cli/src/constructs/alert-channel-codegen.ts (3)
packages/cli/src/constructs/internal/codegen/index.ts (2)
Codegen(1-1)Context(2-2)packages/cli/src/sourcegen/program.ts (1)
Program(7-53)packages/cli/src/constructs/internal/codegen/context.ts (1)
Context(5-121)
packages/cli/src/sourcegen/expr.ts (3)
packages/cli/src/sourcegen/index.ts (10)
ExpressionValue(15-15)Value(33-33)NewExpressionValue(16-16)ArgumentsValue(1-1)args(2-2)Output(28-28)CallExpressionValue(17-17)MemberExpressionValue(18-18)BinaryExpressionValue(19-19)UnaryExpressionValue(20-20)packages/cli/src/sourcegen/args.ts (1)
ArgumentsValue(4-27)packages/cli/src/sourcegen/output.ts (1)
Output(40-85)
packages/cli/src/constructs/check-group-codegen.ts (7)
packages/cli/src/constructs/frequency-codegen.ts (2)
FrequencyResource(4-4)valueForFrequency(6-54)packages/cli/src/constructs/alert-escalation-policy-codegen.ts (2)
AlertEscalationResource(4-4)valueForAlertEscalation(6-65)packages/cli/src/constructs/retry-strategy-codegen.ts (2)
RetryStrategyResource(4-4)valueForRetryStrategy(6-77)packages/cli/src/constructs/internal/codegen/context.ts (1)
Context(5-121)packages/cli/src/constructs/internal/codegen/index.ts (2)
Context(2-2)Codegen(1-1)packages/cli/src/constructs/key-value-pair-codegen.ts (1)
valueForKeyValuePair(4-17)packages/cli/src/constructs/api-check-codegen.ts (1)
valueForAssertion(21-38)
packages/cli/src/constructs/heartbeat-check-codegen.ts (3)
packages/cli/src/constructs/check-codegen.ts (2)
CheckResource(15-33)buildCheckProps(35-144)packages/cli/src/constructs/internal/codegen/index.ts (2)
Codegen(1-1)Context(2-2)packages/cli/src/constructs/internal/codegen/context.ts (1)
Context(5-121)
packages/cli/src/constructs/webhook-alert-channel-codegen.ts (7)
packages/cli/src/constructs/alert-channel-codegen.ts (2)
AlertChannelResource(21-29)buildAlertChannelProps(31-51)packages/cli/src/constructs/key-value-pair-codegen.ts (1)
valueForKeyValuePair(4-17)packages/cli/src/constructs/internal/codegen/index.ts (2)
Codegen(1-1)Context(2-2)packages/cli/src/constructs/incidentio-alert-channel-codegen.ts (1)
IncidentioAlertChannelCodegen(29-62)packages/cli/src/constructs/msteams-alert-channel-codegen.ts (1)
MSTeamsAlertChannelCodegen(14-40)packages/cli/src/constructs/telegram-alert-channel-codegen.ts (1)
TelegramAlertChannelCodegen(28-67)packages/cli/src/constructs/internal/codegen/context.ts (1)
Context(5-121)
packages/cli/src/sourcegen/argsbuilder.ts (2)
packages/cli/src/sourcegen/exprbuilder.ts (4)
build(39-41)context(34-37)ExpressionValueBuilder(11-42)expr(5-9)packages/cli/src/sourcegen/args.ts (1)
ArgumentsValue(4-27)
packages/cli/src/sourcegen/arraybuilder.ts (9)
packages/cli/src/sourcegen/objectbuilder.ts (7)
array(45-47)build(62-64)value(57-60)ObjectValueBuilder(18-65)object(12-16)object(49-51)expr(53-55)packages/cli/src/sourcegen/exprbuilder.ts (4)
build(39-41)context(34-37)ExpressionValueBuilder(11-42)expr(5-9)packages/cli/src/sourcegen/array.ts (1)
ArrayValue(4-31)packages/cli/src/sourcegen/string.ts (1)
StringValue(6-17)packages/cli/src/sourcegen/boolean.ts (1)
BooleanValue(4-15)packages/cli/src/sourcegen/number.ts (1)
NumberValue(4-15)packages/cli/src/sourcegen/null.ts (1)
NullValue(4-8)packages/cli/src/sourcegen/undefined.ts (1)
UndefinedValue(4-8)packages/cli/src/sourcegen/identifier.ts (1)
IdentifierValue(4-15)
packages/cli/src/sourcegen/objectbuilder.ts (9)
packages/cli/src/sourcegen/arraybuilder.ts (7)
object(54-56)build(71-73)value(62-65)ArrayValueBuilder(18-74)array(12-16)array(50-52)expr(58-60)packages/cli/src/sourcegen/exprbuilder.ts (4)
build(39-41)context(34-37)ExpressionValueBuilder(11-42)expr(5-9)packages/cli/src/sourcegen/object.ts (2)
ObjectProperty(4-12)ObjectValue(14-39)packages/cli/src/sourcegen/string.ts (1)
StringValue(6-17)packages/cli/src/sourcegen/boolean.ts (1)
BooleanValue(4-15)packages/cli/src/sourcegen/number.ts (1)
NumberValue(4-15)packages/cli/src/sourcegen/null.ts (1)
NullValue(4-8)packages/cli/src/sourcegen/undefined.ts (1)
UndefinedValue(4-8)packages/cli/src/sourcegen/identifier.ts (1)
IdentifierValue(4-15)
🪛 Biome (1.9.4)
packages/cli/src/constructs/check-codegen.ts
[error] 169-169: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
[error] 171-171: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
[error] 173-173: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
[error] 175-175: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
[error] 177-177: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
packages/cli/src/constructs/webhook-alert-channel-codegen.ts
[error] 102-102: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
[error] 114-114: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
🔇 Additional comments (158)
packages/cli/src/sourcegen/value.ts (1)
3-5: Well-designed base class for value type hierarchy.The abstract
Valueclass establishes a clean, polymorphic interface through therendermethod that all subclasses must implement. This approach provides flexibility while ensuring consistent rendering behavior throughout the source generation system.packages/cli/src/constructs/internal/codegen/index.ts (1)
1-2: Follows standard module export pattern.This index file efficiently re-exports classes from their respective modules, creating a clean API surface and simplifying imports throughout the codebase. This is a good practice for maintainable TypeScript modules.
packages/cli/src/sourcegen/args.ts (1)
4-27: Clean implementation of arguments value rendering.The
ArgumentsValueimplementation correctly extends theValueclass and provides a well-structuredrendermethod. The code properly handles parentheses, comma separation, and whitespace between arguments, including the edge case of the first item.packages/cli/src/constructs/playwright-config-codegen.ts (1)
1-8: Clean and focused implementation of a Playwright config transformer.This code aligns with the source code generation pattern, providing a straightforward way to transform Playwright configurations into the AST-like representation required by the sourcegen system. The implementation is minimal and follows the same pattern used elsewhere in the codebase.
packages/cli/src/constructs/key-value-pair-codegen.ts (1)
4-17: Good implementation of conditional property handling.The function correctly implements conditional logic for optional properties (
lockedandsecret), only including them when they're defined. This prevents undefined values from appearing in the generated code and follows best practices for optional fields.packages/cli/src/constructs/check-group.ts (1)
16-17: Import syntax modernization.The change from
import type { X }toimport { type X }reflects a more modern TypeScript import style. This doesn't affect functionality but helps standardize the codebase's import syntax.packages/cli/package.json (1)
63-65: CLI feature addition for importing resources.The new "import" topic entry in the package.json file properly documents the new import functionality for users. This matches the PR's objective of adding AST-like source code generation capabilities, likely for the purpose of importing existing resources into a project.
packages/cli/src/sourcegen/identifier.ts (1)
1-19: Implementation looks correct and well-structured.The
IdentifierValueclass extendingValuealong with theidenthelper function provides a clean implementation for handling identifier values in the source generation process. The code follows good practices with proper type definitions and clear responsibilities.packages/cli/src/sourcegen/array.ts (1)
1-31: Implementation looks correct and consistent with the source generation pattern.The
ArrayValueclass properly extendsValueand provides appropriate rendering logic for arrays, with special handling for empty arrays.packages/cli/e2e/__tests__/help.spec.ts (2)
26-36: Test coverage for the new import command looks good.The test case properly verifies that the CLI provides appropriate help text for the new import command and its subcommands.
51-52: Additional commands section is correctly updated.The addition of the import command to the additional commands section ensures that users can discover the new functionality through the main help output.
packages/cli/src/sourcegen/object.ts (1)
1-39: Implementation of ObjectValue looks correct and well-structured.The
ObjectPropertyandObjectValueclasses provide a clean implementation for handling object structures in the source generation process.packages/cli/src/constructs/internal/codegen/codegen.ts (1)
4-16: Well-structured abstract class for code generationThis abstract class serves as a solid foundation for implementing various code generators, with a clear interface for subclasses to follow. The generic type parameter
Tprovides flexibility for different resource types.packages/cli/src/sourcegen/string.ts (1)
1-23: LGTM! StringValue implementation is clean and follows the Value patternThe implementation properly extends the Value class and provides appropriate rendering functionality. It follows the same pattern as other Value implementations in the system.
packages/cli/src/sourcegen/unknown.ts (1)
1-41: Well-designed recursive value conversion utilityThis function provides a robust way to convert arbitrary JavaScript values into the appropriate Value objects. The recursive handling of arrays and objects is particularly well-implemented.
packages/cli/src/sourcegen/program.ts (1)
1-54: LGTM! Program class provides a good foundation for code generationThe Program class effectively manages imports and code sections, with a clean render method to produce the final output. Overall, this is a well-designed central component for the source generation system.
packages/cli/src/constructs/browser-check-codegen.ts (2)
6-12: Clear and well-structured interface definition.The
BrowserCheckResourceinterface properly extendsCheckResourceand defines all necessary properties for browser checks. The typings are appropriate, and optional properties are correctly marked.
16-46: Implementation of the code generation is complete and follows established patterns.The
BrowserCheckCodegenclass correctly extends the baseCodegenclass and implements thegencodemethod as expected. The method properly imports the necessary construct, builds the program structure, and includes all the required properties. The call tobuildCheckPropsensures that all properties from the baseCheckResourceinterface are included.packages/cli/src/commands/import/commit.ts (1)
12-68: Well-structured implementation of the commit command.The overall structure of the
ImportCommitCommandclass is well designed. It extendsAuthCommand, defines appropriate flags, and implements a clear workflow for selecting and committing import plans. The use of prompts for user interaction is well implemented, and there's appropriate error handling for the case where a plan ID is missing from the plan map.packages/cli/src/commands/import/cancel.ts (1)
12-112: Well-designed command implementation with good user experience.The
ImportCancelCommandclass is well structured and provides a good user experience with clear options and feedback. The implementation correctly handles cancellation of multiple plans, exiting without changes, and error scenarios. The addition of an "all plans" option is especially useful.packages/cli/src/constructs/dashboard-codegen.ts (3)
4-27: Comprehensive interface with well-defined properties.The
DashboardResourceinterface defines a complete set of dashboard properties with appropriate types and optionality markers. The interface is well-organized and should cover all needed dashboard configuration options.
31-139: Clean and consistent implementation of the dashboard code generation.The
DashboardCodegenclass follows the established pattern for code generation in this codebase. It properly handles all the properties defined in the interface, checking if each is defined before adding it to the generated code. The use of the builder pattern makes the code structure clear and maintainable.
84-136: Consistent handling of boolean properties.The code uses
!== undefinedchecks for boolean properties, which is the correct approach since it allows for explicitly setfalsevalues to be included in the generated code.packages/cli/src/constructs/frequency-codegen.ts (1)
1-54: Well-structured implementation for frequency code generationThe implementation cleanly handles both numeric frequency values and Frequency objects, with proper detection of predefined constants. Good use of type safety with the union type FrequencyResource.
packages/cli/src/constructs/telegram-alert-channel-codegen.ts (1)
26-67: Well-structured implementation for Telegram alert channel generationThe implementation correctly extracts necessary information from config, handles conditional properties appropriately, and follows the Codegen pattern established in the codebase.
packages/cli/src/constructs/retry-strategy-codegen.ts (3)
39-58: 'FIXED' and 'LINEAR' strategies have identical implementationsBoth strategies are using the
fixedStrategybuilder. This might be intentional but could be confusing without a comment explaining why LINEAR uses fixedStrategy.Are 'FIXED' and 'LINEAR' retry strategies expected to have identical implementations? If intentional, consider adding a comment explaining this design choice.
case 'LINEAR': return expr(ident('RetryStrategyBuilder'), builder => { + // LINEAR strategy uses fixedStrategy internally as they share the same implementation builder.member(ident('fixedStrategy')) builder.call(builder => { builder.object(builder => { buildCommonOptions(strategy, builder) }) }) })
6-38: Well-structured implementation with good handling of edge casesThe implementation correctly handles null/undefined cases and provides a helper function for common options, making the code more maintainable.
59-77: Good error handling for unsupported typesThe implementation provides appropriate error handling for unsupported strategy types, which helps with debugging and prevents silent failures.
packages/cli/src/constructs/heartbeat-check-codegen.ts (1)
1-29: Clean implementation of heartbeat check code generationThe implementation correctly extends the Codegen class, properly sets required properties, and reuses code through the buildCheckProps utility. The structure follows the established patterns in the codebase.
packages/cli/src/constructs/maintenance-window-codegen.ts (1)
4-12: Well-structured resource interface.This interface clearly defines all the necessary fields for maintaining a maintenance window resource in code generation, ensuring a strong contract for future usage.
packages/cli/src/constructs/multi-step-check-codegen.ts (1)
6-11: Extending fromCheckResourceis a good approach.Including multi-step specifics on top of a shared base streamlines code generation for checks, reducing duplication.
packages/cli/src/sourcegen/decl.ts (4)
5-7: Proper use of an abstract class.Having an abstract
Declarationbase class provides a clean extension point for specialized declarations.
9-20: No immediate concerns withIdentifierDeclaration.The straightforward rendering of the identifier looks consistent with the rest of the code generation framework.
26-52: Variable declaration logic is clear and succinct.Using
letorconstbased on amutableflag cleanly handles different assignment scenarios without introducing unnecessary complexity.
54-66:ExportDeclarationusage is consistent.Wrapping a declaration with export logic is a neat way to keep declaration rendering separate from export handling.
packages/cli/src/constructs/private-location-group-assignment-codegen.ts (1)
1-17: Clean implementation of the private location group assignment code generation.The implementation correctly registers private location group assignments in the context and appropriately indicates that there's no code to generate for this resource type. The class extends Codegen with proper typing and follows the established pattern for resource codegen classes.
packages/cli/src/sourcegen/output.ts (2)
1-11: OutputBuffer implementation looks good.The OutputBuffer class provides a clean way to accumulate string chunks and combine them. The implementation is straightforward and effective.
13-38: OutputLine implementation is well structured.The OutputLine class properly manages line content and indentation. The collect method correctly handles indentation by repeating spaces based on the level.
packages/cli/src/constructs/alert-escalation-policy-codegen.ts (3)
1-8: Good setup of alert escalation resource type and imports.The file properly imports necessary components from sourcegen and defines the AlertEscalationResource type.
9-35: Well structured helper function for common arguments.The appendCommonArguments helper function nicely encapsulates the common configuration options for both escalation types, avoiding code duplication. The function correctly handles undefined checks for the optional properties.
37-65: Comprehensive handling of different escalation types.The switch statement properly handles the different escalation types with appropriate error handling for unsupported types. The implementation generates the correct expressions based on the escalation type and its properties.
packages/cli/src/commands/import/apply.ts (1)
1-21: Command setup looks good.The ImportApplyCommand class correctly extends AuthCommand and provides appropriate configuration for the command description and flags.
packages/cli/src/constructs/api-check-codegen.ts (9)
1-7: Imports and basic setup look correct.
No issues stand out here. The usage ofCodegen,Context, and other imports is consistent, and the structure follows typical TypeScript conventions.
8-19: Interface design is clear and extensible.
TheApiCheckResourceinterface cleanly extendsCheckResourceand adds API-specific fields. The commented-out TODO lines for script dependencies indicate future expansion.
21-38: Robust assertion value generator.
Your switch handles each assertion source thoroughly, and unrecognized sources are rejected with a clear error. This ensures strong type safety and error handling. Great job tying into the existing numeric vs. general assertion logic.
40-42: Constant declaration is straightforward.
Declaringconstructas'ApiCheck'is clear and easy to understand for subsequent usage.
43-73: Well-structured request property builder.
Withingencode, constructing therequestobject is done via a clear builder pattern. Optional fields likeipFamily,followRedirects, andskipSSLare handled gracefully. The loop forassertionsis also intuitive and consistent with your assertion codegen logic.
75-99: Body, headers, and query parameters inclusion is consistent.
LeveragingvalueForKeyValuePairto handle headers and query parameters ensures DRY logic. Everything is straightforward in building up these properties.
101-108: Basic auth integration is simple but lacks validation.
While the code currently allows empty Username/Password fields, this might be acceptable depending on your requirements. If you require non-empty credentials, consider adding a quick check or warning.Do we expect any specific constraints (e.g., non-empty credentials)? If so, verifying this before code generation would prevent error-prone configurations.
140-146: Degraded and max response times handled well.
Using optional checks forresource.degradedResponseTimeandresource.maxResponseTimeensures these fields are only set when relevant.
148-149: Seamless integration with shared check props.
InvokingbuildCheckPropsfinalizes the API check resource generation without redundant logic. This indicates a good separation of concerns.packages/cli/src/constructs/internal/assertion-codegen.ts (2)
4-43: Comprehensive numeric assertion builder.
Your switch statement covers all numeric comparison cases. Parsing the target as an integer prevents unexpected type mismatches. The default case ensures unknown comparisons are rejected early.
45-146: Flexible general assertion builder.
You’ve addressed a wide range of comparison types (like key/value checks, null checks, emptiness checks). The fallback error handling for unknown comparisons is robust. This uniform approach simplifies expansions if new assertions emerge.packages/cli/src/constructs/private-location-check-assignment-codegen.ts (3)
3-6: Resource interface is minimal and focused.
ThePrivateLocationCheckAssignmentResourceinterface precisely captures the two key fields. This keeps it straightforward for downstream logic.
8-11: Preparation phase registers the assignment.
UsingregisterPrivateLocationCheckAssignmentensures the context is immediately aware of the relationship between private location and check, which keeps everything in sync.
13-16: No code generation needed for this resource.
Having a no-opgencodeis acceptable here, given it’s purely a relational mapping. This is clean and unambiguous.packages/cli/src/constructs/slack-alert-channel-codegen.ts (3)
5-11: Interface structure looks correct and consistent.
TheSlackAlertChannelResourceinterface cleanly extendsAlertChannelResourceand mandatesurl, which is essential for Slack integrations. The inclusion of an optionalchannelmaintains flexibility across different Slack workspaces.
16-18: Registration prefix is consistent with other alert channel codegens.
Using'slackAlert'as a prefix aligns with the standard naming convention seen in sister classes (e.g.,emailAlert). This ensures that variable naming in the context remains coherent and predictable.
20-43: Sufficient usage of shared alert channel properties.
CallingbuildAlertChannelProps(builder, resource)guarantees that common alert channel behaviors (like SSL expiry checks) are uniformly applied. This design is consistent with the overall codegen architecture.packages/cli/src/constructs/email-alert-channel-codegen.ts (3)
5-10: Interface mirrors standard patterns for alert channels.
TheEmailAlertChannelResourceneatly extendsAlertChannelResourcewith atypeand a simpleconfigobject. This design follows the pattern established by other alert channel resource interfaces.
15-17: Consistent registration of email alert channels.
Registering the resource with a'emailAlert'prefix keeps it parallel with Slack and future channels, simplifying the context lookups.
19-36: Proper layering via shared builder logic.
LeveragingbuildAlertChannelProps(builder, resource)ensures consistent handling of universal alert channel fields (like recovery/failure flags). The overall code generation approach is coherent and maintainable.packages/cli/src/constructs/alert-channel-subscription-codegen.ts (3)
3-7: Flexible subscription resource structure.
Allowing bothgroupIdandcheckIdto be optional ensures that subscriptions can be created for individual checks or entire groups. This design is flexible for different alerting needs.
9-18: Accurate registration logic but consider edge cases.
Registering check and/or group subscriptions inprepareis appropriate. However, if bothgroupIdandcheckIdare defined, ensure that subscribing an alert channel to both simultaneously is intended and doesn’t cause duplication.Would you like to confirm that subscribing the same channel to a check and a group is desired, or should we prevent double-subscriptions?
20-23: No generation required.
Leavinggencodeempty is valid since no code needs to be generated for mere subscriptions. This approach separates side effects (subscriptions) from code emission.packages/cli/src/constructs/check-codegen.ts (2)
15-33: Comprehensive check resource interface.
TheCheckResourceincludes a thorough set of optional fields, handling advanced configurations (e.g.,frequency,alertSettings,retryStrategy). This design cleanly accommodates multiple check types in one interface.
35-144: Robust property construction with graceful error handling.
- The
buildCheckPropsfunction systematically covers all check fields.- The try-catch blocks for
privateLocationIdsandalertChannelIdsallow for non-blocking fallback behavior, though silently ignoring errors could mask unexpected issues.Consider logging or clarifying such errors to avoid confusion in future debugging scenarios.
packages/cli/src/commands/import/plan.ts (1)
18-27: No immediate issues with the command definition.
The class definition correctly sets flags and description.packages/cli/src/constructs/tcp-check-codegen.ts (2)
14-25: Logic is consistent with existing assertion code.
This function correctly integrates with other assertion-generating utilities.
29-71: Well-structured TCP check code generation.
UsingbuildCheckPropskeeps property handling consistent across different check types.packages/cli/src/constructs/sms-alert-channel-codegen.ts (2)
5-11: Consider phone number validation for SMS alert channels.
You may want to validate or sanitize the phone number before generating code to prevent errors or logs containing invalid PII.Would you like a script to check for existing phone number validation code or relevant tests?
20-43: Implementation is consistent with other alert channel codegen classes.
No issues detected in the approach for generating the SMS alert channel.packages/cli/src/sourcegen/declbuilder.ts (3)
1-4: Well-structured importsThe imports are clean and focused, bringing in only the necessary components from related files.
5-9: Good implementation of the builder patternThe
declfunction provides a clean API for creating declarations using a builder pattern. The function signature is clear and the implementation is concise.
11-33: Well-designed builder class with proper encapsulationThe
DeclarationBuilderclass follows good design principles:
- Uses private fields (with
#prefix) for proper encapsulation- Implements a fluent interface by returning
thisfrom modifier methods- Clear and focused methods that handle specific aspects of declaration building
- Clean separation of concerns between variable declaration and export functionality
packages/cli/src/constructs/msteams-alert-channel-codegen.ts (3)
6-10: Clean interface extensionThe interface properly extends
WebhookAlertChannelResourceand correctly specifies the webhook type as 'WEBHOOK_MSTEAMS'.
14-18: Consistent registration pattern in prepare methodThe
preparemethod follows the established pattern of registering the alert channel with a descriptive prefix ('teamsAlert'), which helps maintain consistency across different alert channel types.
19-39: Well-structured code generationThe
gencodemethod implements a clean approach to generating code for MS Teams alert channels:
- Imports the necessary construct
- Retrieves the context-managed identifier
- Uses the declaration builder in a consistent manner
- Correctly includes the required properties (name, url)
- Reuses the common
buildAlertChannelPropsfunction for shared alert propertiespackages/cli/src/constructs/phone-call-alert-channel-codegen.ts (4)
5-11: Well-defined resource interfaceThe interface clearly defines the structure for phone call alert channels with appropriate types and property requirements. The optional
nameproperty is correctly marked with a question mark.
15-18: Consistent alert channel registrationThe
preparemethod maintains consistency with other alert channel implementations by using a descriptive prefix ('phoneCallAlert') when registering.
32-34: Proper handling of optional propertiesThe code correctly handles the optional
nameproperty with a conditional block, ensuring it's only included in the generated code when present.
20-43: Consistent code generation patternThe overall code generation follows the established pattern across alert channel types, making the codebase more maintainable and predictable.
packages/cli/src/constructs/private-location-codegen.ts (4)
4-10: Clean resource interface definitionThe interface clearly defines all required properties for a private location with appropriate types. Optional properties (
iconandproxyUrl) are correctly marked with question marks.
14-17: Simple and clear registrationThe
preparemethod follows a consistent pattern with other codegen classes, registering the private location with its ID.
32-38: Proper handling of multiple optional propertiesThe code correctly handles both optional properties (
iconandproxyUrl) with separate conditional blocks, ensuring they're only included in the generated code when present.
19-43: Consistent code generation implementationThe
gencodemethod maintains the same structure and approach used across other codegen classes, enhancing maintainability through consistent patterns:
- Imports the necessary construct
- Retrieves the context-managed identifier
- Builds declarations with the same pattern
- Properly handles required and optional properties
packages/cli/src/constructs/opsgenie-alert-channel-codegen.ts (3)
1-13: Well-structured interface definitionThe
OpsgenieAlertChannelResourceinterface correctly extendsAlertChannelResourceand includes all the necessary configuration properties for an OpsGenie alert channel. The type is properly defined as a string literal 'OPSGENIE', which ensures type safety when working with different alert channel types.
17-20: Consistent alert channel registration patternThe
preparemethod follows the established pattern for alert channel registration, using a descriptive prefix 'opsgenieAlert' which will help generate readable variable names in the output code.
22-44: Well-structured code generation implementationThe
gencodemethod follows a consistent pattern for generating source code for OpsGenie alert channels. The implementation correctly:
- Imports the required construct
- Looks up the alert channel ID from the context
- Extracts the configuration
- Generates the appropriate AST nodes using the builder pattern
- Includes all required configuration properties
- Applies the common alert channel properties
This implementation will produce clean, consistent code for OpsGenie alert channels.
packages/cli/src/constructs/pagerduty-alert-channel-codegen.ts (3)
5-12: Properly defined interface with optional propertiesThe
PagerdutyAlertChannelResourceinterface is well-structured, correctly extending the baseAlertChannelResourceand defining the required 'PAGERDUTY' type. The config object properly marksaccountandserviceNameas optional while makingserviceKeyrequired.
16-19: Consistent alert channel registration patternThe
preparemethod correctly registers the alert channel with the 'pagerdutyAlert' prefix, which maintains consistency with other alert channel implementations.
21-48: Robust code generation with proper handling of optional propertiesThe
gencodemethod correctly handles both required and optional configuration properties:
- It only includes
accountandserviceNamewhen they are defined- It always includes the required
serviceKeyproperty- It properly applies the common alert channel properties using
buildAlertChannelPropsThis conditional approach ensures that the generated code is clean and only includes relevant properties.
packages/cli/src/sourcegen/exprbuilder.ts (2)
5-9: Well-designed expression builder factory functionThe
exprfunction provides a clean way to create and configure expression values using the builder pattern. This approach enables fluent, readable code when generating expressions.
11-41: Comprehensive expression building capabilitiesThe
ExpressionValueBuilderclass provides a complete set of methods for building different types of expressions:
newfor constructing new expressionscallfor method callsmemberfor property accesscontextandbuildfor managing the builder stateThe implementation properly handles the builder context throughout method chaining and supports all the core expression types needed for code generation.
packages/cli/src/constructs/incidentio-alert-channel-codegen.ts (3)
7-11: Clear webhook type constraint with proper interface extensionThe
IncidentioAlertChannelResourceinterface correctly extends theWebhookAlertChannelResourceand constrains thewebhookTypeto specifically be 'WEBHOOK_INCIDENTIO', ensuring type safety when working with Incident.io webhooks.
29-32: Consistent alert channel registration patternThe
preparemethod follows the established pattern for alert channel registration, using a descriptive prefix 'incidentioAlert' for clear variable naming.
34-61: Clean implementation with proper API key extractionThe
gencodemethod correctly:
- Imports the necessary construct
- Retrieves the alert channel ID from the context
- Extracts the configuration
- Conditionally includes the API key when it's available
- Applies common alert channel properties
The code is well-structured and follows established patterns from other alert channel implementations while handling the specific requirements of Incident.io integration.
packages/cli/src/constructs/alert-channel-codegen.ts (9)
1-3: Imports look appropriate.
They properly fetch theCodegenandContextclasses from internal modules and theProgramandObjectValueBuilderfromsourcegen. No immediate issues.
4-10: Channel-specific codegen imports are well-organized.
Importing each channel’s codegen in a separate statement is clear and maintainable.
12-19: Union type enumeration is clear.
The definedAlertChannelTypeunion accurately captures the channel options. If additional channels are introduced in the future, be sure to update both this type and the corresponding codegens.
21-29: Interface structure is cohesive.
All relevant fields (sendRecovery,sendFailure, etc.) are included, and numeric/boolean fields are adequately represented. Theid: numberproperty is good for internal reference.
31-51: Property builder function is consistent.
By conditionally checking forundefined, the code avoids generating superfluous fields. The logic is readable and avoids confusion between falsey booleans and the truly undefined case.
53-62: Class definition extends common codegen pattern.
Declaring channel-specific codegens and grouping them intocodegensByTypesimplifies lookups. No issues noted.
63-82: Constructor initialization is well-structured.
Each channel codegen is instantiated and correctly indexed incodegensByType. This is straightforward and maintainable.
84-91:preparemethod design is intuitive.
The method properly dispatches to the relevant channel codegen based on resource type, throwing a clear error if unsupported.
93-100:gencodedispatch method is straightforward.
Mirrors theprepareflow, ensuring the correct codegen is used. This consistent structure helps ensure minimal duplication.packages/cli/src/sourcegen/index.ts (1)
1-33: Central re-exports are well-managed.
This “barrel” file conveniently consolidates exports for the variousValueand builder classes. The naming scheme is consistent, and the exports are straightforward.packages/cli/src/constructs/construct-codegen.ts (9)
1-3: Imports from internal and source modules are correct.
They align with the recognized class interfaces (Codegen,Context,Program). No concerns here.
4-12: Resource-specific codegen imports are clear.
Bringing in each specialized codegen for top-level resource handling is well-structured.
14-23:ResourceTypeunion is extensible.
Similar toAlertChannelType, it’s straightforward to add new resource types. Keep it updated if new resource categories are added.
31-41:resourceOrderimproves sorting but inverts typical ascending logic.
return bo - ao;sorts resources in descending order. If that’s intended for an internal ordering priority, it’s fine. Otherwise, confirm the desired sort direction.Do you intend to sort from highest to lowest priority or would ascending order be preferable?
43-49:sortResourcesis straightforward.
The function leveragesresourceOrderand returns a consistently sorted list. The logic is readable and easy to extend when new resource types are introduced.
51-61: Class fields centralize resource codegens.
The constructor references them. No immediate issues. This design fosters single entry for all resource type code generation.
63-86: Constructor organizes specialized codegens properly.
Objects are instantiated for each resource type and stored in a clear record mapping. Straightforward approach.
88-96:preparemethod logic is consistent.
Ensures the correct codegen is used based onresource.type, relying on a uniform signature. The error message is descriptive.
97-105:gencodedispatch is symmetrical toprepare.
Handles the same resource-based codegen lookup for generation. The uniform pattern keeps logic consistent across methods.packages/cli/src/rest/projects.ts (7)
27-27: No code to review
No content was added besides a blank line. Nothing to discuss.
39-49: Looks good!
The new interfaces for import plan changes and import plans look structurally sound.
81-84: Consider handling potential errors
Creating import plans is straightforward, but consider how to handle or log errors if the server responds with failures.
94-101: Listing import plans appears correct
This implementation is consistent with thefindImportPlansapproach and remains straightforward.
103-105: Looks good
The DELETE request approach for canceling an import plan is well-structured.
107-109: No issues spotted
The method is straightforward and consistent with typical REST design patterns for applying an import plan.
111-113: Implementation is appropriate
Similarly, committing an import plan aligns well with established patterns.packages/cli/src/sourcegen/expr.ts (5)
5-7: Base abstract class is fine
The abstractExpressionValueprovides a good foundation for specialized expressions.
8-24: Clean 'new' Expression Implementation
This neatly constructs a 'new' expression, combining the callee and arguments with appropriate spacing.
26-40: Function call expression is correct
Implementation is consistent with typical call patterns. Therendermethod is straightforward.
59-80: Binary expression operator coverage
The chosen operators cover many common scenarios. Adding parentheses for complex nesting might be beneficial in certain contexts, but this approach is acceptable for now.
82-98: Unary expression checks out
Implementation follows standard patterns for unary operators and rendering.packages/cli/src/constructs/check-group-codegen.ts (2)
1-41: Resource interfaces look comprehensive
Interfaces forBrowserCheckConfigResource,MultiStepCheckConfigResource, andCheckGroupResourceare well-defined and align with the overall check structure.
229-253: Code generation logic is well-structured
The class cleanly registers the resource and generates the final code snippet using the builder pattern. No major issues identified.packages/cli/src/constructs/webhook-alert-channel-codegen.ts (5)
1-11: Good use of imports and type definitions
The initial imports cleanly separate concerns for building alert channel props and advanced code generation. Everything appears consistent.
12-31: Interfaces provide clear structure
DefiningWebhookAlertChannelResourceConfigand extendingAlertChannelResourceforWebhookAlertChannelResourcelooks coherent and keeps the data model consistent with other alert channel resources.
33-74: Streamlined config builder
buildWebhookAlertChannelConfigfollows a consistent pattern to conditionally set fields. This approach is modular and maintainable.
76-83: Construct naming
Usingconst construct = 'WebhookAlertChannel'ensures the final code references a consistent resource name.
84-95: Delegation to specialized codegens
Mapping distinct webhook types to their specialized classes is a sound approach that promotes separation of concerns.packages/cli/src/sourcegen/objectbuilder.ts (3)
1-11: Imports and class references
All imports are relevant for building objects, expressions, and handling special value types. Code is nicely organized.
12-16: Reusable object construction
Theobject(...)helper function effectively encapsulates object building and returns a fully realizedValue.
18-65: Fluent interface for object creation
ObjectValueBuilder’s methods handle varied property types cleanly. This approach improves readability and modularity.packages/cli/src/sourcegen/arraybuilder.ts (3)
1-11: Imports set the stage for array building
All dependencies align well with generating arrays of differentValuesubclasses.
12-16: array(...) factory
EncapsulatingArrayValueBuildercreation behindarray(...)fosters a clean, fluent interface.
18-74: Fluent array builder
ArrayValueBuilderprovides a robust, chainable way to add various value types. Theempty()approach is a nice convenience.packages/cli/src/sourcegen/argsbuilder.ts (16)
1-11: Imports look good.
They provide all necessary dependencies for this builder pattern without introducing unused references or circular dependencies.
13-17: Theargsfunction is concise and straightforward.
This function effectively encapsulates the builder instantiation and usage, returning the constructedArgumentsValue. No issues identified.
19-19: Comment on circular dependency avoidance.
It's good practice to avoid extendingArrayValueBuilderhere to prevent circular imports. This provides a clear separation of responsibilities for argument values versus array values.
20-21: Private field for builder elements is well-scoped.
Using a private array#elementskeeps the implementation details hidden while providing a controlled interface for adding and retrieving values.
23-26:empty()method ensures a quick reset.
This is a convenient way to clear the builder state without re-instantiating the class.
28-34: String & boolean value additions.
The code is straightforward, and callingthis.value(...)maintains a consistent pattern for pushing new values into#elements.
36-38: Number value addition is correct.
No off-by-one or type issues spotted.
40-42: Null value handling is correct.
This method properly adds aNullValueinstance into the builder.
44-46: Undefined value handling is consistent.
Maintains the same approach as thenull()method.
48-50: Identifier values are properly supported.
This ensures references to named identifiers (e.g., variable names) are added to the builder array.
52-54:array(build: ...)usage.
Delegating array construction toarray(build)helps keep the code modular and avoids duplication.
56-58:object(build: ...)usage.
Similar toarray(), nicely delegates object construction to a dedicated builder function.
60-62:exprmethod is a neat way to build expressions.
Leverages theexprhelper, allowing for more complex expressions within arguments.
64-67:value()acts as a generic insertion point.
This gives maximum flexibility for pushing anyValueimplementation into the builder.
69-71:elementsgetter provides read-only access.
Useful for external examination or debugging without exposing direct writes to#elements.
73-75:build()method finalizes the argument values.
Returns a newArgumentsValueinstance containing the accumulated values. This completes the builder pattern cleanly.packages/cli/src/constructs/internal/codegen/context.ts (1)
5-121: Overall Context class design is clear and maintains good data integrity.
- Each register/lookup method pairs well, storing and retrieving identifiers in private
Mapfields.- The usage of numeric vs. string keys is consistent with the domain logic.
- Error handling on undefined mappings is correct, though a more descriptive error (see above) is recommended.
| if (resource.sslCheckDomain) { | ||
| builder.string('sslCheckDomain', resource.sslCheckDomain) | ||
| } |
There was a problem hiding this comment.
Potential null handling issue with sslCheckDomain.
The condition if (resource.sslCheckDomain) will not handle cases where sslCheckDomain is explicitly set to null, even though the interface allows for null as a valid value.
Modify the condition to explicitly check for undefined:
- if (resource.sslCheckDomain) {
+ if (resource.sslCheckDomain !== undefined && resource.sslCheckDomain !== null) {Committable suggestion skipped: line range outside the PR's diff.
| async run (): Promise<void> { | ||
| const { flags } = await this.parse(ImportPlanCommand) | ||
| const { | ||
| config: configFilename, | ||
| } = flags | ||
|
|
||
| const { configDirectory, configFilenames } = splitConfigFilePath(configFilename) | ||
| const { | ||
| config: checklyConfig, | ||
| } = await loadChecklyConfig(configDirectory, configFilenames) | ||
|
|
||
| const { | ||
| logicalId, | ||
| } = checklyConfig | ||
|
|
||
| const { data: existingPlans } = await api.projects.findImportPlans(logicalId, { | ||
| onlyUncommitted: true, | ||
| }) | ||
|
|
||
| if (existingPlans.length !== 0) { | ||
| await this.#handleExistingPlans(existingPlans) | ||
| } | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
Add unit tests to handle existing import plans scenario.
Consider verifying the user flow when existing plans are detected and the user chooses different prompt options.
Run the following script to locate relevant tests:
🏁 Script executed:
#!/bin/bash
rg -A 15 "ImportPlanCommand" testsLength of output: 80
Action Required: Add Unit Tests for Handling Existing Import Plans
The changes in packages/cli/src/commands/import/plan.ts (lines 29–51) currently have no accompanying tests. We need to add unit tests to validate the code paths when existing import plans are detected. In particular, please verify that:
- The flow through
#handleExistingPlansis correctly triggered when there are non-zeroexistingPlans. - Different user prompt responses (e.g., accepting, canceling, or modifying actions) yield the expected outcomes.
- Edge cases and potential error conditions are properly handled.
The earlier search using rg -A 15 "ImportPlanCommand" tests did not find any test files (indicating a missing tests directory). Please address this by creating relevant tests to ensure robustness.
sorccu
force-pushed
the
simokinnunen/sc-23506/cli-import-existing-resources
branch
from
8e0a955 to
470b019
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (37)
packages/cli/src/constructs/telegram-alert-channel-codegen.ts (2)
12-17: Consider making the API key extraction more robustThe
apiKeyFromUrlfunction uses a specific regex pattern that assumes a fixed URL format. This might be brittle if the Telegram API URL structure changes in the future.Consider adding:
- More comprehensive regex pattern matching
- Error handling for malformed URLs
- Documentation about the expected URL format
function apiKeyFromUrl (url: string): string | undefined { - const match = /https:\/\/api.telegram.org\/bot([^/]+)\/sendMessage/.exec(url) + // Match both standard and custom domain Telegram bot API URLs + const match = /https:\/\/(?:api\.telegram\.org|[^/]+)\/bot([^/]+)\/sendMessage/.exec(url) if (match) { return match[1] } + // Log warning or return specific error value for debugging }
19-24: Add error handling for chatId extractionThe
chatIdFromTemplatefunction extracts the chat ID without validating if it's a valid Telegram chat ID format.function chatIdFromTemplate (template: string): string | undefined { const match = /chatId=(-?[0-9]+)/.exec(template) if (match) { + // Ensure we have a valid chat ID format (group IDs are negative numbers, user IDs are positive) + const chatId = match[1] + if (!/^-?[0-9]+$/.test(chatId)) { + return undefined + } return match[1] } }packages/cli/src/sourcegen/decl.ts (1)
38-52: Consider centralizing whitespace handlingThe
rendermethod inVariableDeclarationhas several calls to manage whitespace that could be abstracted to reduce duplication and improve consistency across the codebase.Consider refactoring the whitespace handling into helper methods:
render (output: Output): void { if (this.options?.mutable) { output.append('let') } else { output.append('const') } output.significantWhitespace() this.identifier.render(output) - output.cosmeticWhitespace() - output.append('=') - output.cosmeticWhitespace() + output.appendWithSpacing('=') // A new helper method that adds spaces around a token this.value.render(output) output.endLine() }packages/cli/src/constructs/private-location-group-assignment-codegen.ts (1)
13-16: Consider adding documentation for the empty implementationThe
gencodemethod has an eslint-disable comment to suppress unused variable warnings, but it would be helpful to add more context about why this resource doesn't generate code.// eslint-disable-next-line @typescript-eslint/no-unused-vars gencode (logicalId: string, resource: PrivateLocationGroupAssignmentResource, context: Context): void { - // Nothing to generate for this resource. + // This resource only establishes a relationship in the context + // but doesn't generate standalone code. The relationship will be used + // when generating code for associated private locations and groups. }packages/cli/src/constructs/internal/assertion-codegen.ts (1)
45-146: Consolidate duplicate code between assertion functionsThe
valueForGeneralAssertionandvalueForNumericAssertionfunctions contain significant duplication in their structure and many comparison operations. Consider refactoring to reduce duplication.You could create a common helper function to handle the shared logic:
function createAssertionValue( klass: string, method: string, comparison: string, target: string | number, property?: string, isNumeric = false ): Value { return expr(ident(klass), builder => { builder.member(ident(method)) builder.call(builder => { if (property) { builder.string(property) } }) // Handle the comparison methods const methodMap: Record<string, string> = { 'EQUALS': 'equals', 'NOT_EQUALS': 'notEquals', // Add all other mappings } const methodName = methodMap[comparison] if (!methodName) { throw new Error(`Unsupported comparison ${comparison}`) } builder.member(ident(methodName)) builder.call(builder => { if (['IS_EMPTY', 'NOT_EMPTY', 'IS_NULL', 'NOT_NULL'].includes(comparison)) { builder.empty() } else if (isNumeric) { builder.number(Number(target)) } else { builder.string(target.toString()) } }) }) }Then refactor both functions to use this helper.
packages/cli/src/commands/import/apply.ts (2)
37-39: Handle scenarios with zero unapplied plans.When
unappliedPlansis empty, callingthis.#selectPlan(unappliedPlans)may lead to a confusing prompt or an index-out-of-bounds error. Consider short-circuiting ifunappliedPlans.length === 0, displaying a message, and returning early.
84-87: Permit an alternate flow after exit selection.After the user selects "Exit without applying," the command terminates abruptly. Consider providing an option for the user to return to the main menu or reselect, if consistent with the CLI design.
packages/cli/src/constructs/api-check-codegen.ts (1)
8-19: Validate optional script fields with caution.
localSetupScript,setupScriptPath,localTearDownScript, andtearDownScriptPathare optional but may overlap. Consider clarifying in documentation or code whether both scripts and script paths can coexist and how each is prioritized.packages/cli/src/constructs/slack-alert-channel-codegen.ts (2)
5-11: Ensure user-provided Slack URL is valid.
config.urlis mandatory, but there's no validation to ensure it is a valid Slack webhook URL. If relevant to your product's usage, consider validating or error handling for malformed URLs.
20-43: Consider user feedback on Slack channel configuration.While the code properly sets the
channelfield, some Slack webhooks may ignore non-default channels if the incoming webhook is restricted. It might be beneficial to add a note or fallback if the Slack response indicates that the specified channel is invalid or restricted.packages/cli/src/constructs/tcp-check-codegen.ts (1)
7-12: Clarify optional response time properties.
degradedResponseTimeandmaxResponseTimeare optional. Be sure to document how the default behavior works when these fields are unspecified, e.g., no performance-based alerts.packages/cli/src/constructs/sms-alert-channel-codegen.ts (1)
20-43: Consider adding a fallback or error handling for invalid phone numbers.
Right now, the code writesconfig.numberasphoneNumberwithout verifying whetherconfig.numbermeets any format requirements. While it may be sufficient as-is, you could include optional formatting or at least a note in the code to handle potential invalid inputs.If you’d like, here’s a potential snippet to indicate a naive check (though you may replace it with a more robust validator later):
builder.string('phoneNumber', config.number) + // Example naive check (not necessarily recommended for production) + // if (!/^\+?\d+$/.test(config.number)) { + // throw new Error('Invalid phone number format') + // }packages/cli/src/sourcegen/declbuilder.ts (1)
20-23: Overwriting behavior may lead to confusion.
Each time.variable(...)is called, the existing declaration in#declis replaced. Ensure this is always the desired behavior, as one might accidentally chain multiple.variable(...)calls expecting them to accumulate rather than overwrite.packages/cli/src/constructs/phone-call-alert-channel-codegen.ts (1)
16-19: Unused parameter “logicalId”.The
logicalIdparameter is never used in thepreparemethod. If it's not necessary for registration in future enhancements, consider removing it for clarity.- prepare (logicalId: string, resource: PhoneCallAlertChannelResource, context: Context): void { + prepare (_logicalId: string, resource: PhoneCallAlertChannelResource, context: Context): void { context.registerAlertChannel(resource.id, 'phoneCallAlert') }packages/cli/src/constructs/check-codegen.ts (1)
35-144: Consider breaking up the large function into smaller helpers.
buildCheckPropsmethod is quite expansive. Partitioning sections (e.g., location handling, alert channel lookups) into smaller helpers can improve maintainability.function buildCheckProps ( program: Program, builder: ObjectValueBuilder, resource: CheckResource, context: Context, ): void { + // Example: create a "buildLocationProps" helper to handle lines 59-66 & 68-82 + // create a "buildAlertProps" helper to handle lines 115-129 ... }packages/cli/src/constructs/opsgenie-alert-channel-codegen.ts (3)
5-13: Potential risk in storing API keys.The
apiKeyfield is stored in plain text. If you plan to generate code that includes this key, consider storing secrets securely or providing usage instructions that clarify the risks of embedding sensitive data.
17-20: Unused parameter “logicalId”.Similar to other codegen classes, the
logicalIdparameter is not used here. Remove it or rename it for clarity if not needed for future expansions.- prepare (logicalId: string, resource: OpsgenieAlertChannelResource, context: Context): void { + prepare (_logicalId: string, resource: OpsgenieAlertChannelResource, context: Context): void { context.registerAlertChannel(resource.id, 'opsgenieAlert') }
22-44: Add codegen test coverage for Opsgenie channels.Introducing a test that verifies the generated
OpsgenieAlertChannelcode will help detect future regressions. You can mock or stub out any secret data likeapiKey.packages/cli/src/constructs/pagerduty-alert-channel-codegen.ts (2)
5-12: Consider adding JSDoc for clarity
The interface is straightforward, but adding a short doc comment (e.g., describing the config fields) could help future contributors understand its usage.
21-48: Ensure sensitive keys are handled securely
While generating code for theserviceKey, confirm that storing secrets in plain text is acceptable. Consider referencing environment variables or a secrets manager to avoid exposing sensitive credentials.packages/cli/src/sourcegen/exprbuilder.ts (4)
5-9: Add documentation
Theexprfunction is central to building and chaining expressions. A brief docstring could help clarify its usage and parameters.
18-22: Method name 'new' may be confusing
Using “new” as a method name can overshadow the language keyword. Consider renaming it to something likecreateNewfor clarity.Here’s a possible diff:
- new (build: (builder: ArgumentsValueBuilder) => void): this { + createNew (build: (builder: ArgumentsValueBuilder) => void): this { const builder = new ArgumentsValueBuilder() build(builder) return this.context(new NewExpressionValue(this.#context, builder.build())) }
30-32: Nested expressions
Themembermethod looks good, but consider adding tests for multi-level member calls to ensure chaining works as expected.
34-37: Potential naming conflict
context(value)might be confused with the existingcontextparameter in the top-levelexprfunction. A more specific name could improve readability.packages/cli/src/constructs/incidentio-alert-channel-codegen.ts (2)
13-25: Consider usingtoLowerCase()
Instead oftoLocaleLowerCase(), which may behave differently across locales, usingtoLowerCase()is often clearer for case-insensitive matching of “authorization” headers.Here’s a possible diff:
- if (header.key.toLocaleLowerCase() !== 'authorization') { + if (header.key.toLowerCase() !== 'authorization') { continue }
34-61: Handling theapiKey
Embedding the extractedapiKeydirectly in generated code can pose security risks if it’s sensitive. Consider referencing environment variables or a secrets manager to keep credentials out of source control.packages/cli/src/constructs/check-group-codegen.ts (2)
72-77: Error is ignored when private location lookup fails.
Catching and ignoring errors may be intentional, but it risks silent failures if contexts are not properly registered. Consider logging or rethrowing for better traceability, if that’s a concern.
110-115: Similar pattern with alert channels.
As with private locations, catching and ignoring context lookup errors is consistent but could mask issues. Logging such errors may improve diagnostics.packages/cli/src/constructs/construct-codegen.ts (1)
43-49:sortResourcesfunction is straightforward.
Sorting logic is easy to follow, but a small comment clarifying the reversed ranking would reduce confusion.packages/cli/src/rest/projects.ts (3)
39-49: Consider adding documentation for these interfaces.
Adding brief doc comments can help clarify the intended usage ofImportPlanandImportPlanChanges.
85-92: Optional parameter documentation.
Consider documentingonlyUnappliedandonlyUncommittedto help clarify their effect for future maintainers.
94-101: Potential shared logic withfindImportPlans.
listImportPlansis very similar tofindImportPlans; consider extracting common logic if you foresee further expansions in filtering or parameters.packages/cli/src/sourcegen/expr.ts (1)
42-57: Handle bracket notation for member expressions.
A TODO note implies there may be tricky cases with dynamic property names. Let me know if you need assistance implementing bracket notation vs dot notation.packages/cli/src/sourcegen/arraybuilder.ts (1)
18-74:ArrayValueBuilderimplementation is consistent and flexible.
- Methods for adding different value types (e.g.,
null(),undefined(),ident()) are clear.- The
empty()method is a helpful convenience for re-initializing the builder.- Consider adding doc comments for each method to maintain clarity on usage and side effects.
packages/cli/src/sourcegen/argsbuilder.ts (1)
19-76:ArgumentsValueBuilderis thorough and readable.
- The method suite (
string(),object(),expr(), etc.) covers a broad range of argument types.- The
empty()method allows for resetting the argument list seamlessly.- As with other builders, doc comments would be useful for describing each method’s intended usage.
packages/cli/src/constructs/internal/codegen/context.ts (2)
3-4: Provide a default error message or accept a custom one.Although this error class extends
Error, the constructor does not supply any default or custom message. Providing a descriptive message can aid in troubleshooting and enhance clarity for debugging.Here’s a possible revision:
-export class MissingContextVariableMappingError extends Error {} +export class MissingContextVariableMappingError extends Error { + constructor(message?: string) { + super(message || 'Missing context variable mapping') + this.name = 'MissingContextVariableMappingError' + } +}
6-16: Consider adding documentation for private fields.All your private maps look good in terms of scoping, but adding JSDoc comments specifying their purpose would improve maintainability and help readers understand at a glance how each mapping is used.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (65)
packages/cli/e2e/__tests__/help.spec.ts(2 hunks)packages/cli/package.json(1 hunks)packages/cli/src/commands/import/apply.ts(1 hunks)packages/cli/src/commands/import/cancel.ts(1 hunks)packages/cli/src/commands/import/commit.ts(1 hunks)packages/cli/src/commands/import/plan.ts(1 hunks)packages/cli/src/constructs/alert-channel-codegen.ts(1 hunks)packages/cli/src/constructs/alert-channel-subscription-codegen.ts(1 hunks)packages/cli/src/constructs/alert-escalation-policy-codegen.ts(1 hunks)packages/cli/src/constructs/api-check-codegen.ts(1 hunks)packages/cli/src/constructs/browser-check-codegen.ts(1 hunks)packages/cli/src/constructs/check-codegen.ts(1 hunks)packages/cli/src/constructs/check-group-codegen.ts(1 hunks)packages/cli/src/constructs/check-group.ts(1 hunks)packages/cli/src/constructs/construct-codegen.ts(1 hunks)packages/cli/src/constructs/dashboard-codegen.ts(1 hunks)packages/cli/src/constructs/email-alert-channel-codegen.ts(1 hunks)packages/cli/src/constructs/frequency-codegen.ts(1 hunks)packages/cli/src/constructs/heartbeat-check-codegen.ts(1 hunks)packages/cli/src/constructs/incidentio-alert-channel-codegen.ts(1 hunks)packages/cli/src/constructs/internal/assertion-codegen.ts(1 hunks)packages/cli/src/constructs/internal/codegen/codegen.ts(1 hunks)packages/cli/src/constructs/internal/codegen/context.ts(1 hunks)packages/cli/src/constructs/internal/codegen/index.ts(1 hunks)packages/cli/src/constructs/key-value-pair-codegen.ts(1 hunks)packages/cli/src/constructs/maintenance-window-codegen.ts(1 hunks)packages/cli/src/constructs/msteams-alert-channel-codegen.ts(1 hunks)packages/cli/src/constructs/multi-step-check-codegen.ts(1 hunks)packages/cli/src/constructs/opsgenie-alert-channel-codegen.ts(1 hunks)packages/cli/src/constructs/pagerduty-alert-channel-codegen.ts(1 hunks)packages/cli/src/constructs/phone-call-alert-channel-codegen.ts(1 hunks)packages/cli/src/constructs/playwright-config-codegen.ts(1 hunks)packages/cli/src/constructs/playwright-config.ts(0 hunks)packages/cli/src/constructs/private-location-check-assignment-codegen.ts(1 hunks)packages/cli/src/constructs/private-location-codegen.ts(1 hunks)packages/cli/src/constructs/private-location-group-assignment-codegen.ts(1 hunks)packages/cli/src/constructs/ref-codegen.ts(1 hunks)packages/cli/src/constructs/retry-strategy-codegen.ts(1 hunks)packages/cli/src/constructs/slack-alert-channel-codegen.ts(1 hunks)packages/cli/src/constructs/sms-alert-channel-codegen.ts(1 hunks)packages/cli/src/constructs/tcp-check-codegen.ts(1 hunks)packages/cli/src/constructs/telegram-alert-channel-codegen.ts(1 hunks)packages/cli/src/constructs/webhook-alert-channel-codegen.ts(1 hunks)packages/cli/src/rest/projects.ts(3 hunks)packages/cli/src/sourcegen/args.ts(1 hunks)packages/cli/src/sourcegen/argsbuilder.ts(1 hunks)packages/cli/src/sourcegen/array.ts(1 hunks)packages/cli/src/sourcegen/arraybuilder.ts(1 hunks)packages/cli/src/sourcegen/boolean.ts(1 hunks)packages/cli/src/sourcegen/decl.ts(1 hunks)packages/cli/src/sourcegen/declbuilder.ts(1 hunks)packages/cli/src/sourcegen/expr.ts(1 hunks)packages/cli/src/sourcegen/exprbuilder.ts(1 hunks)packages/cli/src/sourcegen/identifier.ts(1 hunks)packages/cli/src/sourcegen/index.ts(1 hunks)packages/cli/src/sourcegen/null.ts(1 hunks)packages/cli/src/sourcegen/number.ts(1 hunks)packages/cli/src/sourcegen/object.ts(1 hunks)packages/cli/src/sourcegen/objectbuilder.ts(1 hunks)packages/cli/src/sourcegen/output.ts(1 hunks)packages/cli/src/sourcegen/program.ts(1 hunks)packages/cli/src/sourcegen/string.ts(1 hunks)packages/cli/src/sourcegen/undefined.ts(1 hunks)packages/cli/src/sourcegen/unknown.ts(1 hunks)packages/cli/src/sourcegen/value.ts(1 hunks)
💤 Files with no reviewable changes (1)
- packages/cli/src/constructs/playwright-config.ts
🚧 Files skipped from review as they are similar to previous changes (37)
- packages/cli/src/sourcegen/number.ts
- packages/cli/src/sourcegen/null.ts
- packages/cli/src/sourcegen/undefined.ts
- packages/cli/e2e/tests/help.spec.ts
- packages/cli/src/constructs/ref-codegen.ts
- packages/cli/src/sourcegen/args.ts
- packages/cli/src/sourcegen/boolean.ts
- packages/cli/src/sourcegen/identifier.ts
- packages/cli/src/constructs/internal/codegen/index.ts
- packages/cli/src/sourcegen/value.ts
- packages/cli/package.json
- packages/cli/src/constructs/check-group.ts
- packages/cli/src/constructs/playwright-config-codegen.ts
- packages/cli/src/commands/import/cancel.ts
- packages/cli/src/sourcegen/object.ts
- packages/cli/src/sourcegen/string.ts
- packages/cli/src/constructs/key-value-pair-codegen.ts
- packages/cli/src/sourcegen/unknown.ts
- packages/cli/src/commands/import/commit.ts
- packages/cli/src/constructs/dashboard-codegen.ts
- packages/cli/src/constructs/internal/codegen/codegen.ts
- packages/cli/src/constructs/multi-step-check-codegen.ts
- packages/cli/src/constructs/alert-escalation-policy-codegen.ts
- packages/cli/src/constructs/heartbeat-check-codegen.ts
- packages/cli/src/commands/import/plan.ts
- packages/cli/src/constructs/frequency-codegen.ts
- packages/cli/src/constructs/browser-check-codegen.ts
- packages/cli/src/constructs/maintenance-window-codegen.ts
- packages/cli/src/sourcegen/output.ts
- packages/cli/src/constructs/retry-strategy-codegen.ts
- packages/cli/src/constructs/private-location-check-assignment-codegen.ts
- packages/cli/src/sourcegen/program.ts
- packages/cli/src/constructs/email-alert-channel-codegen.ts
- packages/cli/src/sourcegen/index.ts
- packages/cli/src/sourcegen/array.ts
- packages/cli/src/constructs/alert-channel-subscription-codegen.ts
- packages/cli/src/constructs/private-location-codegen.ts
🧰 Additional context used
🧬 Code Definitions (21)
packages/cli/src/constructs/api-check-codegen.ts (5)
packages/cli/src/constructs/check-codegen.ts (2)
CheckResource(15-33)buildCheckProps(35-144)packages/cli/src/constructs/internal/assertion-codegen.ts (2)
valueForNumericAssertion(4-43)valueForGeneralAssertion(45-146)packages/cli/src/constructs/internal/codegen/index.ts (2)
Codegen(1-1)Context(2-2)packages/cli/src/constructs/internal/codegen/context.ts (1)
Context(5-121)packages/cli/src/constructs/key-value-pair-codegen.ts (1)
valueForKeyValuePair(4-17)
packages/cli/src/constructs/opsgenie-alert-channel-codegen.ts (3)
packages/cli/src/constructs/alert-channel-codegen.ts (2)
AlertChannelResource(21-29)buildAlertChannelProps(31-51)packages/cli/src/constructs/internal/codegen/index.ts (2)
Codegen(1-1)Context(2-2)packages/cli/src/constructs/internal/codegen/context.ts (1)
Context(5-121)
packages/cli/src/constructs/telegram-alert-channel-codegen.ts (4)
packages/cli/src/constructs/webhook-alert-channel-codegen.ts (2)
WebhookAlertChannelResource(28-31)WebhookAlertChannelResourceConfig(17-26)packages/cli/src/constructs/internal/codegen/index.ts (2)
Codegen(1-1)Context(2-2)packages/cli/src/constructs/internal/codegen/context.ts (1)
Context(5-121)packages/cli/src/constructs/alert-channel-codegen.ts (1)
buildAlertChannelProps(31-51)
packages/cli/src/constructs/msteams-alert-channel-codegen.ts (4)
packages/cli/src/constructs/webhook-alert-channel-codegen.ts (2)
WebhookAlertChannelResource(28-31)WebhookAlertChannelResourceConfig(17-26)packages/cli/src/constructs/internal/codegen/index.ts (2)
Codegen(1-1)Context(2-2)packages/cli/src/constructs/internal/codegen/context.ts (1)
Context(5-121)packages/cli/src/constructs/alert-channel-codegen.ts (1)
buildAlertChannelProps(31-51)
packages/cli/src/sourcegen/decl.ts (3)
packages/cli/src/sourcegen/output.ts (1)
Output(40-85)packages/cli/src/sourcegen/index.ts (6)
Output(28-28)IdentifierValue(23-23)VariableDeclaration(7-7)Value(33-33)ExportDeclaration(8-8)decl(11-11)packages/cli/src/sourcegen/declbuilder.ts (1)
decl(5-9)
packages/cli/src/constructs/check-group-codegen.ts (7)
packages/cli/src/constructs/frequency-codegen.ts (2)
FrequencyResource(4-4)valueForFrequency(6-54)packages/cli/src/constructs/alert-escalation-policy-codegen.ts (2)
AlertEscalationResource(4-4)valueForAlertEscalation(6-65)packages/cli/src/constructs/retry-strategy-codegen.ts (2)
RetryStrategyResource(4-4)valueForRetryStrategy(6-77)packages/cli/src/constructs/internal/codegen/index.ts (2)
Context(2-2)Codegen(1-1)packages/cli/src/constructs/internal/codegen/context.ts (1)
Context(5-121)packages/cli/src/constructs/key-value-pair-codegen.ts (1)
valueForKeyValuePair(4-17)packages/cli/src/constructs/api-check-codegen.ts (1)
valueForAssertion(21-38)
packages/cli/src/constructs/slack-alert-channel-codegen.ts (2)
packages/cli/src/constructs/alert-channel-codegen.ts (2)
AlertChannelResource(21-29)buildAlertChannelProps(31-51)packages/cli/src/constructs/internal/codegen/context.ts (1)
Context(5-121)
packages/cli/src/constructs/construct-codegen.ts (12)
packages/cli/src/constructs/internal/codegen/index.ts (2)
Codegen(1-1)Context(2-2)packages/cli/src/constructs/alert-channel-codegen.ts (1)
AlertChannelCodegen(53-101)packages/cli/src/constructs/alert-channel-subscription-codegen.ts (1)
AlertChannelSubscriptionCodegen(9-24)packages/cli/src/constructs/check-codegen.ts (1)
CheckCodegen(146-182)packages/cli/src/constructs/check-group-codegen.ts (1)
CheckGroupCodegen(231-252)packages/cli/src/constructs/dashboard-codegen.ts (1)
DashboardCodegen(31-140)packages/cli/src/constructs/maintenance-window-codegen.ts (1)
MaintenanceWindowCodegen(16-64)packages/cli/src/constructs/private-location-codegen.ts (1)
PrivateLocationCodegen(14-44)packages/cli/src/constructs/private-location-check-assignment-codegen.ts (1)
PrivateLocationCheckAssignmentCodegen(8-17)packages/cli/src/constructs/private-location-group-assignment-codegen.ts (1)
PrivateLocationGroupAssignmentCodegen(8-17)packages/cli/src/sourcegen/program.ts (1)
Program(7-53)packages/cli/src/constructs/internal/codegen/context.ts (1)
Context(5-121)
packages/cli/src/constructs/tcp-check-codegen.ts (5)
packages/cli/src/constructs/check-codegen.ts (2)
CheckResource(15-33)buildCheckProps(35-144)packages/cli/src/sourcegen/program.ts (1)
Program(7-53)packages/cli/src/constructs/internal/assertion-codegen.ts (2)
valueForGeneralAssertion(45-146)valueForNumericAssertion(4-43)packages/cli/src/constructs/internal/codegen/index.ts (2)
Codegen(1-1)Context(2-2)packages/cli/src/constructs/internal/codegen/context.ts (1)
Context(5-121)
packages/cli/src/constructs/pagerduty-alert-channel-codegen.ts (3)
packages/cli/src/constructs/alert-channel-codegen.ts (2)
AlertChannelResource(21-29)buildAlertChannelProps(31-51)packages/cli/src/constructs/internal/codegen/index.ts (2)
Codegen(1-1)Context(2-2)packages/cli/src/constructs/internal/codegen/context.ts (1)
Context(5-121)
packages/cli/src/constructs/private-location-group-assignment-codegen.ts (2)
packages/cli/src/constructs/internal/codegen/index.ts (2)
Codegen(1-1)Context(2-2)packages/cli/src/constructs/internal/codegen/context.ts (1)
Context(5-121)
packages/cli/src/constructs/phone-call-alert-channel-codegen.ts (3)
packages/cli/src/constructs/alert-channel-codegen.ts (2)
AlertChannelResource(21-29)buildAlertChannelProps(31-51)packages/cli/src/constructs/internal/codegen/index.ts (2)
Codegen(1-1)Context(2-2)packages/cli/src/constructs/internal/codegen/context.ts (1)
Context(5-121)
packages/cli/src/constructs/alert-channel-codegen.ts (9)
packages/cli/src/constructs/internal/codegen/index.ts (2)
Codegen(1-1)Context(2-2)packages/cli/src/constructs/phone-call-alert-channel-codegen.ts (1)
PhoneCallAlertChannelCodegen(15-44)packages/cli/src/constructs/email-alert-channel-codegen.ts (1)
EmailAlertChannelCodegen(14-37)packages/cli/src/constructs/opsgenie-alert-channel-codegen.ts (1)
OpsgenieAlertChannelCodegen(17-45)packages/cli/src/constructs/pagerduty-alert-channel-codegen.ts (1)
PagerdutyAlertChannelCodegen(16-49)packages/cli/src/constructs/slack-alert-channel-codegen.ts (1)
SlackAlertChannelCodegen(15-44)packages/cli/src/constructs/sms-alert-channel-codegen.ts (1)
SmsAlertChannelCodegen(15-44)packages/cli/src/constructs/webhook-alert-channel-codegen.ts (1)
WebhookAlertChannelCodegen(78-134)packages/cli/src/sourcegen/program.ts (1)
Program(7-53)
packages/cli/src/sourcegen/expr.ts (2)
packages/cli/src/sourcegen/args.ts (1)
ArgumentsValue(4-27)packages/cli/src/sourcegen/output.ts (1)
Output(40-85)
packages/cli/src/constructs/check-codegen.ts (5)
packages/cli/src/constructs/frequency-codegen.ts (2)
FrequencyResource(4-4)valueForFrequency(6-54)packages/cli/src/constructs/alert-escalation-policy-codegen.ts (2)
AlertEscalationResource(4-4)valueForAlertEscalation(6-65)packages/cli/src/constructs/retry-strategy-codegen.ts (2)
RetryStrategyResource(4-4)valueForRetryStrategy(6-77)packages/cli/src/constructs/internal/codegen/index.ts (2)
Context(2-2)Codegen(1-1)packages/cli/src/constructs/key-value-pair-codegen.ts (1)
valueForKeyValuePair(4-17)
packages/cli/src/sourcegen/argsbuilder.ts (3)
packages/cli/src/sourcegen/index.ts (8)
args(2-2)ArgumentsValueBuilder(2-2)ArgumentsValue(1-1)Value(33-33)array(4-4)object(27-27)ExpressionValueBuilder(22-22)expr(22-22)packages/cli/src/sourcegen/exprbuilder.ts (4)
build(39-41)context(34-37)ExpressionValueBuilder(11-42)expr(5-9)packages/cli/src/sourcegen/args.ts (1)
ArgumentsValue(4-27)
packages/cli/src/sourcegen/objectbuilder.ts (9)
packages/cli/src/sourcegen/arraybuilder.ts (7)
object(54-56)build(71-73)value(62-65)ArrayValueBuilder(18-74)array(12-16)array(50-52)expr(58-60)packages/cli/src/sourcegen/exprbuilder.ts (4)
build(39-41)context(34-37)ExpressionValueBuilder(11-42)expr(5-9)packages/cli/src/sourcegen/object.ts (2)
ObjectProperty(4-12)ObjectValue(14-39)packages/cli/src/sourcegen/string.ts (1)
StringValue(6-17)packages/cli/src/sourcegen/boolean.ts (1)
BooleanValue(4-15)packages/cli/src/sourcegen/number.ts (1)
NumberValue(4-15)packages/cli/src/sourcegen/null.ts (1)
NullValue(4-8)packages/cli/src/sourcegen/undefined.ts (1)
UndefinedValue(4-8)packages/cli/src/sourcegen/identifier.ts (1)
IdentifierValue(4-15)
packages/cli/src/sourcegen/exprbuilder.ts (3)
packages/cli/src/sourcegen/index.ts (8)
expr(22-22)Value(33-33)ExpressionValueBuilder(22-22)ExpressionValue(15-15)ArgumentsValueBuilder(2-2)NewExpressionValue(16-16)CallExpressionValue(17-17)MemberExpressionValue(18-18)packages/cli/src/sourcegen/argsbuilder.ts (4)
expr(60-62)build(73-75)value(64-67)ArgumentsValueBuilder(20-76)packages/cli/src/sourcegen/expr.ts (3)
NewExpressionValue(8-24)CallExpressionValue(26-40)MemberExpressionValue(42-57)
packages/cli/src/constructs/sms-alert-channel-codegen.ts (3)
packages/cli/src/constructs/alert-channel-codegen.ts (2)
AlertChannelResource(21-29)buildAlertChannelProps(31-51)packages/cli/src/constructs/internal/codegen/index.ts (2)
Codegen(1-1)Context(2-2)packages/cli/src/constructs/internal/codegen/context.ts (1)
Context(5-121)
packages/cli/src/constructs/internal/codegen/context.ts (1)
packages/cli/src/constructs/internal/codegen/index.ts (2)
MissingContextVariableMappingError(2-2)Context(2-2)
packages/cli/src/constructs/webhook-alert-channel-codegen.ts (8)
packages/cli/src/constructs/alert-channel-codegen.ts (2)
AlertChannelResource(21-29)buildAlertChannelProps(31-51)packages/cli/src/constructs/key-value-pair-codegen.ts (1)
valueForKeyValuePair(4-17)packages/cli/src/constructs/internal/codegen/index.ts (2)
Codegen(1-1)Context(2-2)packages/cli/src/constructs/incidentio-alert-channel-codegen.ts (1)
IncidentioAlertChannelCodegen(29-62)packages/cli/src/constructs/msteams-alert-channel-codegen.ts (1)
MSTeamsAlertChannelCodegen(14-40)packages/cli/src/constructs/telegram-alert-channel-codegen.ts (1)
TelegramAlertChannelCodegen(28-67)packages/cli/src/sourcegen/program.ts (1)
Program(7-53)packages/cli/src/constructs/internal/codegen/context.ts (1)
Context(5-121)
🪛 Biome (1.9.4)
packages/cli/src/constructs/check-codegen.ts
[error] 169-169: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
[error] 171-171: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
[error] 173-173: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
[error] 175-175: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
[error] 177-177: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
packages/cli/src/constructs/webhook-alert-channel-codegen.ts
[error] 102-102: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
[error] 114-114: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: test - windows-latest
- GitHub Check: test - ubuntu-latest
🔇 Additional comments (76)
packages/cli/src/constructs/private-location-group-assignment-codegen.ts (1)
9-11: LGTM: Appropriate use of context registrationThe
preparemethod correctly registers the private location group assignment with the context using the provided resource properties.packages/cli/src/commands/import/apply.ts (2)
22-31: Consider verifying the presence or validity of the config file before proceeding.If
flags.configis not provided or points to an invalid file,splitConfigFilePathandloadChecklyConfigmay throw an error. Consider either adding a default value or handling this case more gracefully with a user-friendly warning.Would you like me to generate a script to confirm whether
flags.configis being passed and properly validated in other parts of the codebase?
62-65: Fix hardcoded plan numbering.All plans are labeled with
"Plan #1 from ...". This creates ambiguity when multiple plans exist. Update the loop to reference the index or use data from the plan to label them uniquely.Apply a diff similar to the past review comment:
- title: `Plan #1 from ${new Date(plan.createdAt)}`, + title: `Plan #${index + 1} from ${new Date(plan.createdAt)}`,packages/cli/src/constructs/api-check-codegen.ts (2)
21-38: Good use of a switch to handle various assertion sources.The function cleanly maps each
assertion.sourceto an appropriate builder call, and it throws an error for unsupported sources. This is a robust, maintainable approach.
42-153: Check for concurrency or parallel codegen usage.While not strictly an issue here, ensure that this code generation logic will not produce race conditions when used in parallel with other codegen tasks. If codegen resources share state, such as referencing a single
Programinstance across threads, consider adding synchronization or clarifying concurrency assumptions.packages/cli/src/constructs/slack-alert-channel-codegen.ts (1)
15-19: Registering the alert channel at prepare time looks correct.This approach correctly associates the alert channel
physicalIdwith a contextual variable prefixslackAlert. No issues found in the logic.packages/cli/src/constructs/tcp-check-codegen.ts (2)
14-25: Robust error handling for unsupported TCP assertion sources.Throwing an error for an unsupported source ensures undefined behavior is caught early. This is a solid approach.
29-71: Handle edge cases in the request object.If
resource.request.portorresource.request.hostnameis omitted or invalid, the code might break. Ensure that these fields are required or handle them gracefully.packages/cli/src/constructs/sms-alert-channel-codegen.ts (2)
1-4: Imports look clean and follow the established source generation structure.
5-11: Confirm mandatory vs. optional fields for phone number.
It may be beneficial to ensure thatconfig.numberis never empty or null. Since a valid phone number is crucial for SMS-based alerts, consider adding a validation or at least a safeguard to avoid runtime errors.Would you like me to generate a script that searches for references to this interface to confirm that every usage provides a valid
numberfield?packages/cli/src/sourcegen/declbuilder.ts (1)
1-9: Straightforward declaration builder pattern.
This approach nicely encapsulates the creation of declarations, improving readability and maintainability of generated code.packages/cli/src/constructs/msteams-alert-channel-codegen.ts (3)
1-3: Imports look aligned with the existing codegen architecture.
6-10: Resource extension appears appropriate.
Using aWebhookAlertChannelResourcebase for MS Teams is suitable, given it’s a type of webhook with a specificwebhookType.
19-39: Validate the MS Teams webhook details.
Given thatwebhookTypeis'WEBHOOK_MSTEAMS', consider verifying the URL domain format or at least that the URL is not empty to avoid sending incomplete or invalid requests.Would you like me to generate a search script to identify how
urlis used in other places, ensuring consistent handling of webhook URLs for MS Teams?packages/cli/src/constructs/phone-call-alert-channel-codegen.ts (2)
5-13: Interface structure appears correct.The
PhoneCallAlertChannelResourceinterface is suitably extendingAlertChannelResourceand specifying a unique'CALL'type. This aligns well with the existing alert channel setup.
20-43: Recommend adding test coverage.To ensure correctness and prevent regressions, it’s best to add codegen tests for this new alert channel type. You can validate code blocks are correctly generated (logical ID usage, phone number, optional name, etc.).
Would you like me to generate a sample test file to verify the generated code structure for
PhoneCallAlertChannelCodegen?packages/cli/src/constructs/check-codegen.ts (1)
15-33: Interface provides clear coverage of check properties.All relevant fields (e.g.,
activated,muted, etc.) are included. This is comprehensive and easy to extend.packages/cli/src/constructs/pagerduty-alert-channel-codegen.ts (1)
16-19: Looks good
Thepreparemethod correctly registers the alert channel in the context with a clear, descriptive prefix.packages/cli/src/sourcegen/exprbuilder.ts (2)
24-28: LGTM
Thecallmethod’s structure is intuitive and aligns well with the builder pattern.
39-41: Completes the builder pattern
Returning the private#contextstate neatly finishes the expression building flow.packages/cli/src/constructs/incidentio-alert-channel-codegen.ts (2)
7-11: Interface usage
ExtendingWebhookAlertChannelResourcewith a specializedwebhookTypeis a clean approach that maintains generic properties while customizing the incident.io configuration.
29-32: Consistent pattern
Thepreparemethod follows the established approach to register alert channels using a recognizable prefix.packages/cli/src/constructs/alert-channel-codegen.ts (6)
1-10: Imports look consistent and modular.
All imported modules and references appear valid. No issues found with structure or syntax.
12-19: Enum-like union type is clear and exhaustive.
Defining all possible alert channel types in a single union is straightforward and helps prevent invalid inputs at compile time.
21-29: Interface is well-structured.
All required and optional properties are clearly defined. This clearly captures the attributes needed for each alert channel resource.
31-51: Utility function enables easy extension.
buildAlertChannelPropsproperly checks for undefined fields before building them. This is a good pattern to avoid missing or extraneous properties.
53-82: Comprehensive initialization of codegen instances.
Each specific codegen class is instantiated and mapped properly incodegensByType. This enforces full coverage for all supported alert channels.
84-101: Appropriate error handling for unsupported alert channel types.
Throwing an error when encountering an undefined type ensures that silent failures do not occur. This is a robust design decision.packages/cli/src/constructs/check-group-codegen.ts (9)
1-10: Imports refer to relevant modules.
No issues with referencing or structuring these imported classes and functions. They align with the rest of the codebase.
11-19: Config resources capture essential properties.
The browser and multi-step check config resources reflect the expected shape, including optional frequency. This is flexible and future-proof.
21-41:CheckGroupResourcethoroughly covers all properties for check groups.
All known attributes (loyalty, environment variables, concurrency, etc.) are well-defined. Optional fields are properly indicated.
43-48: Function signature is self-explanatory.
buildCheckGroupPropsuses (program, builder, resource, context) as parameters, cleanly separating logic from data.
88-95: Tag array logic is clear.
Looping through tags elegantly handles the possibility of zero or multiple tags.
126-128: Alert escalation policy is clearly integrated.
UsingvalueForAlertEscalationnicely decouples the escalation-specific logic from the main flow.
130-168: Browser and multi-step checks follow consistent patterns.
Frequency and testMatch arrays are properly handled. The code reads cleanly.
222-222: Defaulting retry strategy tovalueForRetryStrategyis robust.
Even if the resource doesn’t specify aretryStrategy, this logic ensures a stable default.
231-253:CheckGroupCodegenextends a solid base.
Usingcontext.registerCheckGroupensures consistent ID tracking. Importing the main construct is done only at gencode time.packages/cli/src/constructs/construct-codegen.ts (6)
1-12: Imports reference the correct codegeneration modules.
No syntactic or logical issues apparent.
14-24:ResourceTypeunion enumerates recognized resource categories.
Useful for controlling the resource handling logic, preventing partial or ambiguous resource types.
31-41: Resource ordering is reversed for higher numbers.
return bo - aosorts descending by default. Confirm that this reversed order is entirely intentional. If you meant an ascending sort, consider swapping.
51-85: Constructor sets up all known resource codegens.
Mapping these resources ensures coverage, just like the approach used inalert-channel-codegen.
88-95: Consistent error-handling approach for unsupported resource types.
By failing fast, you guarantee that the calling code must handle or correct an unknown resource type.
97-104: Delegated generation approach fosters clarity.
Calls to the underlying codegens keep this class minimal and maintainable, avoiding duplication.packages/cli/src/rest/projects.ts (4)
81-83: Looks good.
Creating an import plan with a simple POST request seems straightforward.
103-105: Straightforward cancellation endpoint.
No issues found; the code is concise and clear.
107-109: Straightforward apply endpoint.
Implementation looks correct.
111-113: Straightforward commit endpoint.
Implementation looks correct.packages/cli/src/sourcegen/expr.ts (7)
5-6: Base class approach looks good.
TheExpressionValueclass extendsValueand seems ready for new expression types.
8-24: Constructor and rendering for 'new' expressions.
Implementation is straightforward and consistent with typical AST generation patterns.
26-40: Call expression rendering is correct.
Logic is clean, with the callee followed by arguments.
59-59: Binary operator definition is concise.
Expanding this list in the future is straightforward if you need more operators.
61-80: Binary expression rendering is clear.
No concerns with the spacing and operator usage.
82-82: Unary operator definition is concise.
Additional unary operators can easily be added as needed.
84-98: Unary expression rendering is correct.
Looks good with straightforward usage of theopproperty.packages/cli/src/constructs/webhook-alert-channel-codegen.ts (8)
1-16: New type and interfaces look good.
The introduction ofWebhookTypeis clean, supporting extension with additional webhook integrations in the future.
17-26: Interfaces are well-structured.
WebhookAlertChannelResourceConfigcaptures necessary details for different webhook types.
28-31: Extends AlertChannelResource clearly.
No issues; theWebhookAlertChannelResourcedefinition is concise and easy to maintain.
33-74: Configuration builder is well-implemented.
Conditional checks for optional fields are cleanly handled, and the usage ofObjectValueBuilderis idiomatic.
76-76: Constant declaration is fine.
No concerns with naming or usage ofconstruct.
78-95: Class construction logic is clear.
The mapping ofcodegensByWebhookTypeensures easy extensibility for new webhook types.
97-107: Duplicate comment: remove return in a void function.
As noted previously, returningcodegen.prepare(...)from avoidfunction triggers a static analysis warning. Either remove thereturnor change the function signature.- return codegen.prepare(logicalId, resource, context) + codegen.prepare(logicalId, resource, context) + return🧰 Tools
🪛 Biome (1.9.4)
[error] 102-102: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
109-133: Duplicate comment: remove return in a void function.
As noted previously, returningcodegen.gencode(...)from avoidfunction triggers a static analysis warning. Either remove thereturnor change the function signature.- return codegen.gencode(logicalId, resource, context) + codegen.gencode(logicalId, resource, context) + return🧰 Tools
🪛 Biome (1.9.4)
[error] 114-114: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
packages/cli/src/sourcegen/objectbuilder.ts (3)
1-10: Good import organization and usage.All imported modules appear relevant and are used appropriately to construct different value types. No redundant or missing imports detected.
12-16: Fluent interface forobject(...)is clear and concise.The builder pattern here allows for clean and intuitive object creation. This method effectively initializes the builder, invokes the consumer's custom logic, and returns the final
ObjectValue.
18-65:✅ Verification successful
Builder pattern implementation is robust. Consider documenting usage.
- The
ObjectValueBuilderprovides a solid API for adding various property types, from primitives to nested objects.- Adding short doc comments or JSDoc annotations for each method could improve maintainability and clarify usage.
- The use of private class fields (
#properties) is modern ES syntax. Validate that your Node/TypeScript environment supports it to avoid unexpected runtime issues.
🌐 Web query:
Which Node.js versions natively support JavaScript private class fields (#private)?💡 Result:
Private class fields using the
#syntax are natively supported in Node.js starting from version 12 and later, enabled by default without requiring command-line flags or transpilers like Babel[1][4][7]. This support was introduced through updates to the V8 JavaScript engine (v7.4+), which powers Node.js[4].Key details:
- Node.js 12 (April 2019): First version with native support for private fields[1][7].
- Node.js 14+: Continues support as the feature became standardized (ES2022)[8].
- Runtime behavior: Private fields are enforced at the language level, making them inaccessible via
Object.keys()or other reflection methods[1][5].class Example { #privateField = 42; // Supported in Node.js ≥12 getValue() { return this.#privateField; } }Earlier Node.js versions (≤11) lack support, and attempts to use the syntax will throw syntax errors. Private fields remain experimental in JavaScript until ES2022 standardization, but Node.js implementations have been stable since v12[4][7].
Citations:
- 1: https://thecodebarbarian.com/nodejs-12-private-class-fields.html
- 2: https://community.grafana.com/t/bug-with-js-classes-that-have-private-fields/98707
- 3: https://jamie.build/javascripts-new-private-class-fields.html
- 4: https://v8.dev/features/class-fields
- 5: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Classes/Private_properties
- 6: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Classes
- 7: https://www.sitepoint.com/javascript-private-class-fields/
- 8: https://dev.to/smitterhane/private-class-fields-in-javascript-es2022-3b8
- 9: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Classes/Public_class_fields
Enhance Documentation & Verify Runtime Environment Compatibility
The implementation of the builder pattern is solid and uses modern ES syntax (e.g., private class fields are supported natively in Node.js v12 and later). To improve maintainability and clarify intended usage:
- Consider adding inline JSDoc or doc comments for each method (e.g.,
string,boolean,number, etc.) to clearly explain parameters and functionality.- Verify that your development/production environments are running on Node.js v12 or later to ensure full support for private class fields.
packages/cli/src/sourcegen/arraybuilder.ts (2)
1-10: Imports look coherent.They align well with the intended functionality (building arrays with various value types).
12-16:array(...)function is well-designed.It mirrors the same fluent pattern found in
object(...), ensuring consistency across builders.packages/cli/src/sourcegen/argsbuilder.ts (2)
1-12: Imports and references are consistent with the builder pattern.The references to
ArgumentsValue,ArrayValueBuilder,ObjectValueBuilder, and others establish a unified approach to constructing arguments.
13-17: Theargs(...)function cleanly encapsulates argument construction.It provides a straightforward interface for generating an
ArgumentsValueinstance, mirroring the style of your other builders.packages/cli/src/constructs/internal/codegen/context.ts (7)
17-30: Looks correct for registering and looking up check groups.The logic for generating a sequential variable name (e.g.,
group1,group2, ...) and retrieving it later is straightforward and consistent with the class’s intended design.
32-45: Alert channel registration and lookup logic appears sound.No significant concerns: the pattern is consistent with how check groups are handled. The variable prefix parameter is a nice touch for naming clarity.
47-60: Code for private location registration and lookup is well-structured.The approach of assigning a simple sequential name for each private location is clear and matches the pattern used above.
62-75: Registering and retrieving private locations for check groups is straightforward.No functional issues spotted. Error handling is consistently done by throwing
MissingContextVariableMappingErrorwhen needed.
77-90: Registering and retrieving private locations for checks is consistent.Good reuse of the approach in the previous method. No immediate issues with performance or correctness.
92-105: Alert channel check subscription logic looks clean.The pattern of appending to an array and retrieving it later is consistent with the existing approach.
107-120: Group alert channel subscription is implemented consistently.The logic closely mirrors earlier patterns, ensuring a uniform API for lookups and assignments. No further concerns.
| gencode (logicalId: string, resource: TelegramAlertChannelResource, context: Context): void { | ||
| this.program.import(construct, 'checkly/constructs') | ||
|
|
||
| const id = context.lookupAlertChannel(resource.id) | ||
|
|
||
| const { config } = resource | ||
|
|
||
| this.program.section(decl(id, builder => { | ||
| builder.variable(expr(ident(construct), builder => { | ||
| builder.new(builder => { | ||
| builder.string(logicalId) | ||
| builder.object(builder => { | ||
| builder.string('name', config.name) | ||
|
|
||
| if (config.template) { | ||
| const chatId = chatIdFromTemplate(config.template) | ||
| if (chatId) { | ||
| builder.string('chatId', chatId) | ||
| } | ||
| } | ||
|
|
||
| if (config.headers) { | ||
| const apiKey = apiKeyFromUrl(config.url) | ||
| if (apiKey) { | ||
| builder.string('apiKey', apiKey) | ||
| } | ||
| } | ||
|
|
||
| buildAlertChannelProps(builder, resource) | ||
| }) | ||
| }) | ||
| })) | ||
| })) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve error handling for URL and template extraction
The gencode method doesn't handle cases where extraction of chatId or apiKey fails but the corresponding configuration exists.
Consider adding more defensive code:
if (config.template) {
const chatId = chatIdFromTemplate(config.template)
if (chatId) {
builder.string('chatId', chatId)
+ } else {
+ // Handle the case where template exists but chatId couldn't be extracted
+ console.warn(`Could not extract chatId from template: ${config.template}`)
}
}
if (config.headers) {
const apiKey = apiKeyFromUrl(config.url)
if (apiKey) {
builder.string('apiKey', apiKey)
+ } else {
+ // Handle the case where headers exist but apiKey couldn't be extracted
+ console.warn(`Could not extract API key from URL: ${config.url}`)
}
}📝 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.
| gencode (logicalId: string, resource: TelegramAlertChannelResource, context: Context): void { | |
| this.program.import(construct, 'checkly/constructs') | |
| const id = context.lookupAlertChannel(resource.id) | |
| const { config } = resource | |
| this.program.section(decl(id, builder => { | |
| builder.variable(expr(ident(construct), builder => { | |
| builder.new(builder => { | |
| builder.string(logicalId) | |
| builder.object(builder => { | |
| builder.string('name', config.name) | |
| if (config.template) { | |
| const chatId = chatIdFromTemplate(config.template) | |
| if (chatId) { | |
| builder.string('chatId', chatId) | |
| } | |
| } | |
| if (config.headers) { | |
| const apiKey = apiKeyFromUrl(config.url) | |
| if (apiKey) { | |
| builder.string('apiKey', apiKey) | |
| } | |
| } | |
| buildAlertChannelProps(builder, resource) | |
| }) | |
| }) | |
| })) | |
| })) | |
| } | |
| gencode (logicalId: string, resource: TelegramAlertChannelResource, context: Context): void { | |
| this.program.import(construct, 'checkly/constructs') | |
| const id = context.lookupAlertChannel(resource.id) | |
| const { config } = resource | |
| this.program.section(decl(id, builder => { | |
| builder.variable(expr(ident(construct), builder => { | |
| builder.new(builder => { | |
| builder.string(logicalId) | |
| builder.object(builder => { | |
| builder.string('name', config.name) | |
| if (config.template) { | |
| const chatId = chatIdFromTemplate(config.template) | |
| if (chatId) { | |
| builder.string('chatId', chatId) | |
| } else { | |
| // Handle the case where template exists but chatId couldn't be extracted | |
| console.warn(`Could not extract chatId from template: ${config.template}`) | |
| } | |
| } | |
| if (config.headers) { | |
| const apiKey = apiKeyFromUrl(config.url) | |
| if (apiKey) { | |
| builder.string('apiKey', apiKey) | |
| } else { | |
| // Handle the case where headers exist but apiKey couldn't be extracted | |
| console.warn(`Could not extract API key from URL: ${config.url}`) | |
| } | |
| } | |
| buildAlertChannelProps(builder, resource) | |
| }) | |
| }) | |
| })) | |
| })) | |
| } |
| export class ExportDeclaration { | ||
| decl: Declaration | ||
|
|
||
| constructor (decl: Declaration) { | ||
| this.decl = decl | ||
| } | ||
|
|
||
| render (output: Output): void { | ||
| output.append('export') | ||
| output.significantWhitespace() | ||
| this.decl.render(output) | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
ExportDeclaration should extend Declaration class
The ExportDeclaration class doesn't extend the Declaration abstract class unlike other declaration classes, which breaks the inheritance hierarchy. This could cause issues when handling declarations polymorphically.
-export class ExportDeclaration {
+export class ExportDeclaration extends Declaration {
decl: Declaration
constructor (decl: Declaration) {
+ super()
this.decl = decl
}
render (output: Output): void {
output.append('export')
output.significantWhitespace()
this.decl.render(output)
}
}📝 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.
| export class ExportDeclaration { | |
| decl: Declaration | |
| constructor (decl: Declaration) { | |
| this.decl = decl | |
| } | |
| render (output: Output): void { | |
| output.append('export') | |
| output.significantWhitespace() | |
| this.decl.render(output) | |
| } | |
| } | |
| export class ExportDeclaration extends Declaration { | |
| decl: Declaration | |
| constructor (decl: Declaration) { | |
| super() | |
| this.decl = decl | |
| } | |
| render (output: Output): void { | |
| output.append('export') | |
| output.significantWhitespace() | |
| this.decl.render(output) | |
| } | |
| } |
| case 'LESS_THAN': | ||
| builder.member(ident('lessThan')) | ||
| builder.call(builder => { | ||
| builder.string(assertion.target) | ||
| }) | ||
| break | ||
| case 'GREATER_THAN': | ||
| builder.member(ident('greaterThan')) | ||
| builder.call(builder => { | ||
| builder.string(assertion.target) | ||
| }) | ||
| break |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Handle numeric comparison types correctly in general assertions
The valueForGeneralAssertion function handles LESS_THAN and GREATER_THAN comparisons as string comparisons, which may produce unexpected results when comparing numeric values.
case 'LESS_THAN':
builder.member(ident('lessThan'))
builder.call(builder => {
- builder.string(assertion.target)
+ // Try to determine if target should be treated as a number
+ const numValue = Number(assertion.target)
+ if (!isNaN(numValue)) {
+ builder.number(numValue)
+ } else {
+ builder.string(assertion.target)
+ }
})
breakApply the same approach to the GREATER_THAN case.
Committable suggestion skipped: line range outside the PR's diff.
| export function valueForNumericAssertion<Source extends string> ( | ||
| klass: string, | ||
| method: string, | ||
| assertion: Assertion<Source>, | ||
| ): Value { | ||
| return expr(ident(klass), builder => { | ||
| builder.member(ident(method)) | ||
| builder.call(builder => { | ||
| builder.empty() | ||
| }) | ||
| switch (assertion.comparison) { | ||
| case 'EQUALS': | ||
| builder.member(ident('equals')) | ||
| builder.call(builder => { | ||
| builder.number(parseInt(assertion.target, 10)) | ||
| }) | ||
| break | ||
| case 'NOT_EQUALS': | ||
| builder.member(ident('notEquals')) | ||
| builder.call(builder => { | ||
| builder.number(parseInt(assertion.target, 10)) | ||
| }) | ||
| break | ||
| case 'LESS_THAN': | ||
| builder.member(ident('lessThan')) | ||
| builder.call(builder => { | ||
| builder.number(parseInt(assertion.target, 10)) | ||
| }) | ||
| break | ||
| case 'GREATER_THAN': | ||
| builder.member(ident('greaterThan')) | ||
| builder.call(builder => { | ||
| builder.number(parseInt(assertion.target, 10)) | ||
| }) | ||
| break | ||
| default: | ||
| throw new Error(`Unsupported comparison ${assertion.comparison} for assertion source ${assertion.source}`) | ||
| } | ||
| }) | ||
| } |
There was a problem hiding this comment.
Handle numeric values more flexibly
The valueForNumericAssertion function uses parseInt to convert all targets to integers, which might cause precision issues with floating-point values.
case 'EQUALS':
builder.member(ident('equals'))
builder.call(builder => {
- builder.number(parseInt(assertion.target, 10))
+ builder.number(Number(assertion.target))
})
breakApply the same change to all other numeric comparisons in this function.
📝 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.
| export function valueForNumericAssertion<Source extends string> ( | |
| klass: string, | |
| method: string, | |
| assertion: Assertion<Source>, | |
| ): Value { | |
| return expr(ident(klass), builder => { | |
| builder.member(ident(method)) | |
| builder.call(builder => { | |
| builder.empty() | |
| }) | |
| switch (assertion.comparison) { | |
| case 'EQUALS': | |
| builder.member(ident('equals')) | |
| builder.call(builder => { | |
| builder.number(parseInt(assertion.target, 10)) | |
| }) | |
| break | |
| case 'NOT_EQUALS': | |
| builder.member(ident('notEquals')) | |
| builder.call(builder => { | |
| builder.number(parseInt(assertion.target, 10)) | |
| }) | |
| break | |
| case 'LESS_THAN': | |
| builder.member(ident('lessThan')) | |
| builder.call(builder => { | |
| builder.number(parseInt(assertion.target, 10)) | |
| }) | |
| break | |
| case 'GREATER_THAN': | |
| builder.member(ident('greaterThan')) | |
| builder.call(builder => { | |
| builder.number(parseInt(assertion.target, 10)) | |
| }) | |
| break | |
| default: | |
| throw new Error(`Unsupported comparison ${assertion.comparison} for assertion source ${assertion.source}`) | |
| } | |
| }) | |
| } | |
| export function valueForNumericAssertion<Source extends string> ( | |
| klass: string, | |
| method: string, | |
| assertion: Assertion<Source>, | |
| ): Value { | |
| return expr(ident(klass), builder => { | |
| builder.member(ident(method)) | |
| builder.call(builder => { | |
| builder.empty() | |
| }) | |
| switch (assertion.comparison) { | |
| case 'EQUALS': | |
| builder.member(ident('equals')) | |
| builder.call(builder => { | |
| builder.number(Number(assertion.target)) | |
| }) | |
| break | |
| case 'NOT_EQUALS': | |
| builder.member(ident('notEquals')) | |
| builder.call(builder => { | |
| builder.number(Number(assertion.target)) | |
| }) | |
| break | |
| case 'LESS_THAN': | |
| builder.member(ident('lessThan')) | |
| builder.call(builder => { | |
| builder.number(Number(assertion.target)) | |
| }) | |
| break | |
| case 'GREATER_THAN': | |
| builder.member(ident('greaterThan')) | |
| builder.call(builder => { | |
| builder.number(Number(assertion.target)) | |
| }) | |
| break | |
| default: | |
| throw new Error(`Unsupported comparison ${assertion.comparison} for assertion source ${assertion.source}`) | |
| } | |
| }) | |
| } |
| gencode (logicalId: string, resource: CheckResource, context: Context): void { | ||
| const { checkType } = resource | ||
|
|
||
| switch (checkType) { | ||
| case 'BROWSER': | ||
| return this.browserCheckCodegen.gencode(logicalId, resource as BrowserCheckResource, context) | ||
| case 'API': | ||
| return this.apiCheckCodegen.gencode(logicalId, resource as ApiCheckResource, context) | ||
| case 'TCP': | ||
| return this.tcpCheckCodegen.gencode(logicalId, resource as TcpCheckResource, context) | ||
| case 'MULTI_STEP': | ||
| return this.multiStepCheckCodegen.gencode(logicalId, resource as MultiStepCheckResource, context) | ||
| case 'HEARTBEAT': | ||
| return this.heartbeatCheckCodegen.gencode(logicalId, resource as HeartbeatCheckResource, context) | ||
| default: | ||
| throw new Error(`Unable to generate code for unsupported check type '${checkType}'.`) | ||
| } | ||
| } |
There was a problem hiding this comment.
Remove “return” statements in a void function.
Static analysis indicates returning from a function with return type void is unnecessary. Instead of returning the sub-codegen calls, simply invoke them and break out of the switch.
gencode (logicalId: string, resource: CheckResource, context: Context): void {
const { checkType } = resource
switch (checkType) {
case 'BROWSER':
- return this.browserCheckCodegen.gencode(logicalId, resource as BrowserCheckResource, context)
+ this.browserCheckCodegen.gencode(logicalId, resource as BrowserCheckResource, context)
+ break
case 'API':
- return this.apiCheckCodegen.gencode(logicalId, resource as ApiCheckResource, context)
+ this.apiCheckCodegen.gencode(logicalId, resource as ApiCheckResource, context)
+ break
case 'TCP':
- return this.tcpCheckCodegen.gencode(logicalId, resource as TcpCheckResource, context)
+ this.tcpCheckCodegen.gencode(logicalId, resource as TcpCheckResource, context)
+ break
case 'MULTI_STEP':
- return this.multiStepCheckCodegen.gencode(logicalId, resource as MultiStepCheckResource, context)
+ this.multiStepCheckCodegen.gencode(logicalId, resource as MultiStepCheckResource, context)
+ break
case 'HEARTBEAT':
- return this.heartbeatCheckCodegen.gencode(logicalId, resource as HeartbeatCheckResource, context)
+ this.heartbeatCheckCodegen.gencode(logicalId, resource as HeartbeatCheckResource, context)
+ break
default:
throw new Error(\`Unable to generate code for unsupported check type '\${checkType}'.\`)
}
}📝 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.
| gencode (logicalId: string, resource: CheckResource, context: Context): void { | |
| const { checkType } = resource | |
| switch (checkType) { | |
| case 'BROWSER': | |
| return this.browserCheckCodegen.gencode(logicalId, resource as BrowserCheckResource, context) | |
| case 'API': | |
| return this.apiCheckCodegen.gencode(logicalId, resource as ApiCheckResource, context) | |
| case 'TCP': | |
| return this.tcpCheckCodegen.gencode(logicalId, resource as TcpCheckResource, context) | |
| case 'MULTI_STEP': | |
| return this.multiStepCheckCodegen.gencode(logicalId, resource as MultiStepCheckResource, context) | |
| case 'HEARTBEAT': | |
| return this.heartbeatCheckCodegen.gencode(logicalId, resource as HeartbeatCheckResource, context) | |
| default: | |
| throw new Error(`Unable to generate code for unsupported check type '${checkType}'.`) | |
| } | |
| } | |
| gencode (logicalId: string, resource: CheckResource, context: Context): void { | |
| const { checkType } = resource | |
| switch (checkType) { | |
| case 'BROWSER': | |
| this.browserCheckCodegen.gencode(logicalId, resource as BrowserCheckResource, context) | |
| break | |
| case 'API': | |
| this.apiCheckCodegen.gencode(logicalId, resource as ApiCheckResource, context) | |
| break | |
| case 'TCP': | |
| this.tcpCheckCodegen.gencode(logicalId, resource as TcpCheckResource, context) | |
| break | |
| case 'MULTI_STEP': | |
| this.multiStepCheckCodegen.gencode(logicalId, resource as MultiStepCheckResource, context) | |
| break | |
| case 'HEARTBEAT': | |
| this.heartbeatCheckCodegen.gencode(logicalId, resource as HeartbeatCheckResource, context) | |
| break | |
| default: | |
| throw new Error(`Unable to generate code for unsupported check type '${checkType}'.`) | |
| } | |
| } |
🧰 Tools
🪛 Biome (1.9.4)
[error] 169-169: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
[error] 171-171: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
[error] 173-173: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
[error] 175-175: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
[error] 177-177: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
packages/cli/src/commands/import/apply.ts (2)
22-60: Improve error handling for non-fancy output.The catch block properly handles error visualization for fancy output, but lacks any messaging or feedback for non-fancy mode. Consider adding a log message before throwing the error.
} catch (err) { if (this.fancy) { ux.action.stop('❌') + } else { + this.log('❌ Failed to apply plan') } throw err }
64-65: Use a more user-friendly date format.The date is currently displayed using the default
Dateformatting, which can vary by locale. Consider using a more consistent and readable format.- title: `Plan #${index + 1} from ${new Date(plan.createdAt)}`, + title: `Plan #${index + 1} from ${new Date(plan.createdAt).toLocaleString()}`, value: plan.id,packages/cli/src/constructs/webhook-alert-channel-codegen.ts (4)
79-79: Fix typo in property name.The property name
indicentioCodegenappears to have a typo - it should beincidentioCodegen(missing 't').- indicentioCodegen: IncidentioAlertChannelCodegen + incidentioCodegen: IncidentioAlertChannelCodegen
82-82: Improve type safety by avoidingany.Using
Codegen<any>loses type safety. Consider using a union type of the specific codegen types.- codegensByWebhookType: Record<WebhookType, Codegen<any>> + codegensByWebhookType: Record<WebhookType, IncidentioAlertChannelCodegen | MSTeamsAlertChannelCodegen | TelegramAlertChannelCodegen>
43-43: Remove unnecessary toString() conversion.The
config.urlis defined as a string in the interface, so calling.toString()is redundant.- builder.string('url', config.url.toString()) + builder.string('url', config.url)
97-108: Consider creating a helper method for webhook type codegen lookup.Both
prepare()andgencode()methods have similar code patterns for looking up and delegating to the appropriate codegen. Consider extracting this logic to a helper method.+ private getCodegenForWebhookType(webhookType?: WebhookType): Codegen<any> | undefined { + if (!webhookType) { + return undefined + } + return this.codegensByWebhookType[webhookType] + } prepare (logicalId: string, resource: WebhookAlertChannelResource, context: Context): void { const { webhookType } = resource.config - if (webhookType) { - const codegen = this.codegensByWebhookType[webhookType] - if (codegen) { - codegen.prepare(logicalId, resource, context) - return - } + const codegen = this.getCodegenForWebhookType(webhookType) + if (codegen) { + codegen.prepare(logicalId, resource, context) + return } context.registerAlertChannel(resource.id, 'webhookAlert') }Similarly, update the
gencode()method to use this helper.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/cli/src/commands/import/apply.ts(1 hunks)packages/cli/src/commands/import/commit.ts(1 hunks)packages/cli/src/constructs/webhook-alert-channel-codegen.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/cli/src/commands/import/commit.ts
🧰 Additional context used
🧬 Code Definitions (1)
packages/cli/src/constructs/webhook-alert-channel-codegen.ts (7)
packages/cli/src/constructs/alert-channel-codegen.ts (2)
AlertChannelResource(21-29)buildAlertChannelProps(31-51)packages/cli/src/constructs/key-value-pair-codegen.ts (1)
valueForKeyValuePair(4-17)packages/cli/src/constructs/internal/codegen/index.ts (2)
Codegen(1-1)Context(2-2)packages/cli/src/constructs/incidentio-alert-channel-codegen.ts (1)
IncidentioAlertChannelCodegen(29-62)packages/cli/src/constructs/msteams-alert-channel-codegen.ts (1)
MSTeamsAlertChannelCodegen(14-40)packages/cli/src/constructs/telegram-alert-channel-codegen.ts (1)
TelegramAlertChannelCodegen(28-67)packages/cli/src/constructs/internal/codegen/context.ts (1)
Context(5-121)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test - windows-latest
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
packages/cli/src/constructs/check-codegen.ts (1)
68-82: Consider adding error logging for failed private location lookupsThe code silently ignores errors when looking up private locations. While this might be intentional to make private locations optional, consider at least logging a warning to help with debugging when something unexpected fails.
const privateLocationIds = (() => { try { return context.lookupCheckPrivateLocations(resource.id) } catch (err) { + console.warn(`Failed to lookup private locations for check ${resource.id}:`, err) } })()packages/cli/src/sourcegen/object.ts (2)
8-18: Consider property name and value validation.
The constructor doesn’t validate the property name or value. For added robustness, ensure that invalid or unexpected values do not slip in.
58-88: Watch for trailing commas in rendered output.
The generated code includes a comma after each property. If this output is parsed by tools that disallow trailing commas (e.g., strict JSON parsers), consider conditionally omitting the last comma.packages/cli/src/sourcegen/objectbuilder.ts (2)
12-16: Public helper function is well-structured.
object(...)uses a builder function internally, improving code readability and reusability.
72-74: Consider clarifying structure immutability post-build.
Returning a newObjectValueafter build is typical. Include a note in docs if the resulting object is intended to be immutable or if further modifications can be made.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/cli/src/constructs/check-codegen.ts(1 hunks)packages/cli/src/constructs/retry-strategy-codegen.ts(1 hunks)packages/cli/src/sourcegen/object.ts(1 hunks)packages/cli/src/sourcegen/objectbuilder.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/cli/src/constructs/retry-strategy-codegen.ts
🧰 Additional context used
🧬 Code Definitions (2)
packages/cli/src/constructs/check-codegen.ts (5)
packages/cli/src/constructs/frequency-codegen.ts (2)
FrequencyResource(4-4)valueForFrequency(6-54)packages/cli/src/constructs/alert-escalation-policy-codegen.ts (2)
AlertEscalationResource(4-4)valueForAlertEscalation(6-65)packages/cli/src/constructs/retry-strategy-codegen.ts (2)
RetryStrategyResource(4-4)valueForRetryStrategy(6-77)packages/cli/src/constructs/internal/codegen/index.ts (2)
Context(2-2)Codegen(1-1)packages/cli/src/constructs/key-value-pair-codegen.ts (1)
valueForKeyValuePair(4-17)
packages/cli/src/sourcegen/object.ts (3)
packages/cli/src/sourcegen/index.ts (3)
Value(33-33)ObjectValue(26-26)Output(28-28)packages/cli/src/sourcegen/objectbuilder.ts (1)
value(67-70)packages/cli/src/sourcegen/output.ts (1)
Output(40-85)
🪛 Biome (1.9.4)
packages/cli/src/constructs/check-codegen.ts
[error] 169-169: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
[error] 171-171: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
[error] 173-173: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
[error] 175-175: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
[error] 177-177: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
🔇 Additional comments (26)
packages/cli/src/constructs/check-codegen.ts (7)
15-33: Well-structured interface for check resourcesThe
CheckResourceinterface provides a clean and comprehensive structure for representing different types of check resources. Good use of optional properties to handle various configuration options that might be present in different check types.
35-40: Well-defined function signature with clear parametersThe
buildCheckPropsfunction has a clear purpose with well-typed parameters. The void return type is appropriate since it mutates the passed builder object.
106-113: Good error handling for missing check groupsI appreciate the explicit error handling when a check references a group that isn't being imported. This will provide clear guidance to users about the dependency issue rather than failing silently.
115-143: Robust implementation of alert channel lookup and configurationThe code correctly handles the lookup of alert channels with proper error handling, and conditionally adds configuration properties only when they're defined in the resource.
164-181: Remove "return" statements in a void functionThe function should not return values as its return type is declared as
void. This issue was also identified by static analysis.gencode (logicalId: string, resource: CheckResource, context: Context): void { const { checkType } = resource switch (checkType) { case 'BROWSER': - return this.browserCheckCodegen.gencode(logicalId, resource as BrowserCheckResource, context) + this.browserCheckCodegen.gencode(logicalId, resource as BrowserCheckResource, context) + break case 'API': - return this.apiCheckCodegen.gencode(logicalId, resource as ApiCheckResource, context) + this.apiCheckCodegen.gencode(logicalId, resource as ApiCheckResource, context) + break case 'TCP': - return this.tcpCheckCodegen.gencode(logicalId, resource as TcpCheckResource, context) + this.tcpCheckCodegen.gencode(logicalId, resource as TcpCheckResource, context) + break case 'MULTI_STEP': - return this.multiStepCheckCodegen.gencode(logicalId, resource as MultiStepCheckResource, context) + this.multiStepCheckCodegen.gencode(logicalId, resource as MultiStepCheckResource, context) + break case 'HEARTBEAT': - return this.heartbeatCheckCodegen.gencode(logicalId, resource as HeartbeatCheckResource, context) + this.heartbeatCheckCodegen.gencode(logicalId, resource as HeartbeatCheckResource, context) + break default: throw new Error(`Unable to generate code for unsupported check type '${checkType}'.`) } }🧰 Tools
🪛 Biome (1.9.4)
[error] 169-169: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
[error] 171-171: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
[error] 173-173: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
[error] 175-175: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
[error] 177-177: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
154-162: Clean initialization of code generators in constructorThe initialization of all the required code generators in the constructor is clean and follows the dependency injection pattern well. This approach makes the code more maintainable and testable.
41-144: Property builder is comprehensive and handles edge casesThe implementation covers all properties from the interface definition and handles conditional properties well. The code is robust in dealing with optional values and includes proper error handling for lookups.
packages/cli/src/sourcegen/object.ts (7)
1-3: Imports look good.
No issues found with these import statements.
4-6: Interface is straightforward.
Definingorder?: numberas an optional property is a clear way to store sorting order.
20-21: Sorter type definition is appropriate.
Declaring a dedicated type for your property sorter helps maintain clarity.
22-24: Options interface is concise.
Optionalsortproperty is well-defined for customizing sorting strategy.
26-36: Name-based comparator is correct.
Simple string comparison logic is fine for alphabetical sorting.
38-51: Order-based comparator is well-implemented.
Leverages the optionalorderproperty and defaults to 0 if missing. This is good.
53-56: Order-then-name sorting is coherent.
Clear fallback to name-based sorting when orders match.packages/cli/src/sourcegen/objectbuilder.ts (12)
1-10: Imports are consistent.
No issues found with module references or import paths.
18-25: Builder constructor is clear.
StoringObjectValueOptionsin a private field is straightforward for controlling how the object is built.
26-28: String property method is concise.
UsesStringValueto encapsulate string data.
30-32: Boolean property method is consistent.
Follows the same pattern, ensuring consistent usage of typed value classes.
34-36: Number property handling is good.
No issues found here; straightforward numeric value usage.
38-40: Null property creation is clear.
Sufficiently indicates a null value.
42-44: Undefined value usage is correct.
Allows for explicit undefined representation in the generated object.
46-48: Identifier reference is well-handled.
Encapsulating an identifier in its ownValuetype is a good practice.
50-52: Array building integration is neat.
Supports nested structures by delegating array creation toarraybuilder.
54-56: Object building integration is straightforward.
Consistently delegates nested object creation to theobject(...)helper.
58-65: Expr building is well-structured.
Uses a dedicatedexpr(...)builder for expressions, aligning with the rest of the pattern.
67-70: Value assignment method looks solid.
Pushing a newObjectPropertyfosters consistent property handling across the builder.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
packages/cli/src/constructs/private-location-codegen.ts (1)
1-56: Consider adding documentation to public API elements.While the code is well-structured, adding JSDoc comments to the interface, function, and class would improve developer experience and make the code more maintainable.
+/** + * Interface representing a Private Location resource. + */ export interface PrivateLocationResource { id: string name: string slugName: string icon?: string proxyUrl?: string } const construct = 'PrivateLocation' +/** + * Creates a reference to an existing Private Location using its ID. + * @param program - The program to add imports to + * @param physicalId - The physical ID of the private location + * @returns A Value representing the expression to reference the private location + */ export function valueForPrivateLocationFromId (program: Program, physicalId: string): Value { program.import(construct, 'checkly/constructs') return expr(ident(construct), builder => { builder.member(ident('fromId')) builder.call(builder => { builder.string(physicalId) }) }) } +/** + * Code generator for Private Location resources. + * Handles the generation of TypeScript code for creating Private Location instances. + */ export class PrivateLocationCodegen extends Codegen<PrivateLocationResource> {packages/cli/src/constructs/check-codegen.ts (2)
121-126: Consider enhancing error handling in the lookupCheckAlertChannels call.The current implementation silently catches errors when looking up check alert channels. Consider adding logging or more explicit error handling to help with debugging, especially since this pattern is used for private locations too.
const alertChannelIds = (() => { try { return context.lookupCheckAlertChannels(resource.id) } catch (err) { + // Log or handle the error more explicitly + // console.debug(`No alert channels found for check ${resource.id}`) } })()
70-75: Consider enhancing error handling in the lookupCheckPrivateLocations call.Similar to the alert channels lookup, this code silently catches errors. Adding more explicit error handling or logging would improve debuggability.
const privateLocationIds = (() => { try { return context.lookupCheckPrivateLocations(resource.id) } catch (err) { + // Log or handle the error more explicitly + // console.debug(`No private locations found for check ${resource.id}`) } })()packages/cli/src/constructs/alert-channel-codegen.ts (2)
12-19: Consider using an enum forAlertChannelType.Using a TypeScript enum could help prevent typos and make refactoring or adding new alert channel types more maintainable.
66-95: Consider extracting a helper for retrieving alert channel codegen.Both the
prepareandgencodemethods duplicate the lookup logic inthis.codegensByType[...]. Factoring out a small helper function could simplify code and reduce repetition.packages/cli/src/constructs/check-group-codegen.ts (1)
74-79: Avoid silently ignoring context lookup errors.The code swallows possible
MissingContextVariableMappingErrorwithout logging. If these lookups fail unexpectedly, debugging might be difficult. Consider logging or handling the error more explicitly to surface issues earlier.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/cli/src/constructs/alert-channel-codegen.ts(1 hunks)packages/cli/src/constructs/check-codegen.ts(1 hunks)packages/cli/src/constructs/check-group-codegen.ts(1 hunks)packages/cli/src/constructs/private-location-codegen.ts(1 hunks)
🧰 Additional context used
🧬 Code Definitions (4)
packages/cli/src/constructs/check-codegen.ts (8)
packages/cli/src/constructs/frequency-codegen.ts (2)
FrequencyResource(4-4)valueForFrequency(6-54)packages/cli/src/constructs/retry-strategy-codegen.ts (2)
RetryStrategyResource(4-4)valueForRetryStrategy(6-77)packages/cli/src/constructs/internal/codegen/index.ts (2)
Context(2-2)Codegen(1-1)packages/cli/src/constructs/internal/codegen/context.ts (1)
Context(5-121)packages/cli/src/constructs/private-location-codegen.ts (1)
valueForPrivateLocationFromId(14-23)packages/cli/src/constructs/key-value-pair-codegen.ts (1)
valueForKeyValuePair(4-17)packages/cli/src/constructs/alert-channel-codegen.ts (1)
valueForAlertChannelFromId(55-64)packages/cli/src/constructs/api-check-codegen.ts (2)
ApiCheckCodegen(42-153)ApiCheckResource(8-19)
packages/cli/src/constructs/alert-channel-codegen.ts (10)
packages/cli/src/sourcegen/program.ts (1)
Program(7-53)packages/cli/src/constructs/internal/codegen/index.ts (2)
Codegen(1-1)Context(2-2)packages/cli/src/constructs/phone-call-alert-channel-codegen.ts (1)
PhoneCallAlertChannelCodegen(15-44)packages/cli/src/constructs/email-alert-channel-codegen.ts (1)
EmailAlertChannelCodegen(14-37)packages/cli/src/constructs/opsgenie-alert-channel-codegen.ts (1)
OpsgenieAlertChannelCodegen(17-45)packages/cli/src/constructs/pagerduty-alert-channel-codegen.ts (1)
PagerdutyAlertChannelCodegen(16-49)packages/cli/src/constructs/slack-alert-channel-codegen.ts (1)
SlackAlertChannelCodegen(15-44)packages/cli/src/constructs/sms-alert-channel-codegen.ts (1)
SmsAlertChannelCodegen(15-44)packages/cli/src/constructs/webhook-alert-channel-codegen.ts (1)
WebhookAlertChannelCodegen(78-136)packages/cli/src/constructs/internal/codegen/context.ts (1)
Context(5-121)
packages/cli/src/constructs/check-group-codegen.ts (9)
packages/cli/src/constructs/frequency-codegen.ts (2)
FrequencyResource(4-4)valueForFrequency(6-54)packages/cli/src/constructs/alert-escalation-policy-codegen.ts (2)
AlertEscalationResource(4-4)valueForAlertEscalation(6-65)packages/cli/src/constructs/retry-strategy-codegen.ts (2)
RetryStrategyResource(4-4)valueForRetryStrategy(6-77)packages/cli/src/constructs/internal/codegen/index.ts (2)
Context(2-2)Codegen(1-1)packages/cli/src/constructs/internal/codegen/context.ts (1)
Context(5-121)packages/cli/src/constructs/private-location-codegen.ts (1)
valueForPrivateLocationFromId(14-23)packages/cli/src/constructs/key-value-pair-codegen.ts (1)
valueForKeyValuePair(4-17)packages/cli/src/constructs/alert-channel-codegen.ts (1)
valueForAlertChannelFromId(55-64)packages/cli/src/constructs/api-check-codegen.ts (1)
valueForAssertion(21-38)
packages/cli/src/constructs/private-location-codegen.ts (3)
packages/cli/src/sourcegen/program.ts (1)
Program(7-53)packages/cli/src/constructs/internal/codegen/index.ts (2)
Codegen(1-1)Context(2-2)packages/cli/src/constructs/internal/codegen/context.ts (1)
Context(5-121)
🪛 Biome (1.9.4)
packages/cli/src/constructs/check-codegen.ts
[error] 179-179: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
[error] 181-181: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
[error] 183-183: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
[error] 185-185: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
[error] 187-187: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
🔇 Additional comments (11)
packages/cli/src/constructs/private-location-codegen.ts (6)
1-3: Imports look good!The imports correctly bring in the necessary dependencies from internal code generation utilities and source generation modules.
4-10: Well-defined interface for private location resources.The interface properly defines the required and optional properties for a private location resource, making it clear what data is needed for code generation.
12-12: Good use of constant for construct name.Using a constant for the construct name prevents string duplication and makes maintenance easier.
14-23: Utility function looks good!This utility function correctly generates code to reference an existing private location by ID. It properly imports the necessary construct and builds the expression using the builder pattern.
25-29: Preparation method correctly registers resources.The
preparemethod correctly registers the private location ID in the context, which will be used later during code generation.
30-54: Code generation implementation is well structured.The
gencodemethod:
- Imports the necessary construct
- Looks up the registered private location
- Generates code with proper conditional handling of optional properties
The code follows the builder pattern consistently and handles the optional properties (
iconandproxyUrl) correctly.packages/cli/src/constructs/check-codegen.ts (3)
174-191: Remove "return" statements in a void function.Static analysis indicates returning from a function with return type
voidis unnecessary. Instead of returning the sub-codegen calls, simply invoke them and break out of the switch.gencode (logicalId: string, resource: CheckResource, context: Context): void { const { checkType } = resource switch (checkType) { case 'BROWSER': - return this.browserCheckCodegen.gencode(logicalId, resource as BrowserCheckResource, context) + this.browserCheckCodegen.gencode(logicalId, resource as BrowserCheckResource, context) + break case 'API': - return this.apiCheckCodegen.gencode(logicalId, resource as ApiCheckResource, context) + this.apiCheckCodegen.gencode(logicalId, resource as ApiCheckResource, context) + break case 'TCP': - return this.tcpCheckCodegen.gencode(logicalId, resource as TcpCheckResource, context) + this.tcpCheckCodegen.gencode(logicalId, resource as TcpCheckResource, context) + break case 'MULTI_STEP': - return this.multiStepCheckCodegen.gencode(logicalId, resource as MultiStepCheckResource, context) + this.multiStepCheckCodegen.gencode(logicalId, resource as MultiStepCheckResource, context) + break case 'HEARTBEAT': - return this.heartbeatCheckCodegen.gencode(logicalId, resource as HeartbeatCheckResource, context) + this.heartbeatCheckCodegen.gencode(logicalId, resource as HeartbeatCheckResource, context) + break default: throw new Error(`Unable to generate code for unsupported check type '${checkType}'.`) } }🧰 Tools
🪛 Biome (1.9.4)
[error] 179-179: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
[error] 181-181: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
[error] 183-183: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
[error] 185-185: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
[error] 187-187: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
37-154: Well-structured property building function.The
buildCheckPropsfunction is well-organized, handling all required and optional properties of a check resource appropriately. It properly deals with different types of properties (boolean, string, arrays) and includes good error handling for resource lookups.
17-35: Well-defined interface with complete check properties.The
CheckResourceinterface provides a comprehensive definition of common check properties with appropriate optional markers. This creates a solid foundation for specific check type interfaces to extend.packages/cli/src/constructs/alert-channel-codegen.ts (1)
97-113: Add test coverage for unsupported channel types.Although there’s a runtime error thrown for unsupported alert channel types, consider adding tests to confirm this error path behaves correctly. This ensures future channel additions don't accidentally break or bypass this check.
packages/cli/src/constructs/check-group-codegen.ts (1)
1-263: Overall Implementation Appears Strong.No concurrency or security concerns are evident, and the modular code design aligns well with the existing codegen framework.
Importing itself does work but code generation has not been connected to the commands yet. It requires some massaging to work with API resources.
Includes a few fixes that were needed to make it work.
Simpler and it's easier to find all implementations. Plus functionality can be shared.
It's tied to constructs and that's a better place for it.
Covers private locations and alert channels.
Insertion order is tough to work with when using helpers for common sets of properties. Instead, sort by key alphabetically, or custom order if desired. For now the custom order is used to make sure the name property of a Check is always first.
sorccu
force-pushed
the
simokinnunen/sc-23506/cli-import-existing-resources
branch
from
4021e4a to
a252605
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (41)
packages/cli/src/constructs/private-location-group-assignment-codegen.ts (1)
3-17: Looks good—consider additional validation or tests
This code successfully registers the private location group assignment. Optionally, you could verify whether bothprivateLocationIdandgroupIdpoint to valid, registered entities and add corresponding test coverage.Would you like help creating a test snippet or a helper method to check resource validity before registering?
packages/cli/src/constructs/api-check-codegen.ts (2)
110-123: Avoid potential overwrite of thesetupScriptproperty
If bothlocalSetupScriptandsetupScriptPathare defined, two separate objects forsetupScriptwill be generated, which may lead to unexpected behavior. Consider using anelse iffor one or defining a clear precedence.if (resource.localSetupScript) { // ... } else if (resource.setupScriptPath) { // ... }
125-138: Clarify precedence for thetearDownScriptproperty
Similar to the setup script, defining bothlocalTearDownScriptandtearDownScriptPathwill build twotearDownScriptobjects. Using anelse ifor a deliberate override strategy can reduce confusion.if (resource.localTearDownScript) { // ... } else if (resource.tearDownScriptPath) { // ... }packages/cli/src/commands/import/apply.ts (2)
22-40: Handle Zero-Plan Scenario Gracefully.If
unappliedPlansis empty, the user receives a prompt referencing "0 unapplied plan(s)" with only an Exit option. Consider an early exit or a user-friendly message indicating no plans exist, which would improve the user experience.if (unappliedPlans.length === 0) { + this.log('No unapplied plans found. Exiting without making any changes.') + this.exit(0) }
47-59: Provide Additional Error Context When Plan Application Fails.If
applyImportPlan(plan.id)fails, adding details about the failing plan (e.g. plan ID, creation time) can simplify troubleshooting.try { await api.projects.applyImportPlan(plan.id) } catch (err) { + this.warn(`Failed to apply plan #${plan.id}.`) throw err }packages/cli/src/constructs/slack-alert-channel-codegen.ts (1)
5-11: Consider Validating Slack URL and Channel.While the resource interface is straightforward, additional validation (or user feedback on invalid props) could improve reliability, especially if the URL or channel is malformed.
packages/cli/src/constructs/email-alert-channel-codegen.ts (1)
5-10: Consider Email Address Validation.Currently, any string is accepted as the address. Providing basic format checks or user feedback on invalid addresses can enhance the robustness of the generator.
packages/cli/src/commands/import/plan.ts (4)
18-28: Consider adding a brief doc comment for theImportPlanCommandclass.A short description of the class purpose, key steps performed in the
runmethod, and usage notes can improve maintainability and help other collaborators.
66-79: Handle other HTTP error codes for plan creation.At present, the code specifically checks for a 404 status. Consider handling other status codes (e.g., 401, 403) or providing a default human-readable message to guide end-users. This ensures more complete error handling and improves the user experience.
112-117: Allow customization of the generated output filename.The generated code is written to
./generated.check.tsby default. For improved flexibility, consider adding a flag or configuration parameter to specify a custom filename or directory path.
130-223: Check for batched cancellation to increase performance.The existing approach cancels each plan in sequence. If performance or rate-limiting becomes a concern, consider a batched API or parallel cancellation strategy (if supported by the backend) to handle multiple cancellations more efficiently.
packages/cli/src/constructs/sms-alert-channel-codegen.ts (1)
20-43: Validate phone number formats.Optionally, you could validate or normalize the phone number prior to code generation to prevent invalid phone entries. Although not strictly required, it can help reduce errors in downstream usage.
packages/cli/src/sourcegen/declbuilder.ts (2)
20-23: Consider handling multiple variable calls.
Current design overwrites#decleach timevariable()is called. If the builder is meant to allow multiple variable declarations before a final build, this would discard prior declarations. Clarify if only one variable creation perDeclarationBuilderis intended.
25-28:export()chaining caution.
Wrapping the existing declaration inExportDeclarationrepeatedly could cause nested exports ifexport()is called multiple times. Confirm whether repeated calls should either be disallowed or replaced with a single export operation.packages/cli/src/constructs/msteams-alert-channel-codegen.ts (1)
19-39: Ensureconfigintegrity before code generation.
While code generation proceeds smoothly, consider validatingconfig(e.g.,name,url) before generating the block to avoid runtime issues.packages/cli/src/constructs/phone-call-alert-channel-codegen.ts (1)
20-43: Validate critical fields inconfig.
Consider basic checks or user-friendly error messages for missing or invalid phone numbers (config.number) before generating code, preventing potential runtime failures.packages/cli/src/constructs/check-codegen.ts (3)
70-75: Improve error handling for privateLocationIds lookup.The function silently ignores errors when looking up privateLocationIds. This makes debugging harder if something goes wrong.
Consider adding proper error handling:
const privateLocationIds = (() => { try { return context.lookupCheckPrivateLocations(resource.id) } catch (err) { + // If this is not a MissingContextVariableMappingError, rethrow + if (!(err instanceof MissingContextVariableMappingError)) { + throw err + } + // Otherwise, return undefined to indicate no private locations } })()
121-126: Improve error handling for alertChannelIds lookup.Similar to the privateLocationIds lookup, errors are silently ignored when looking up alertChannelIds.
Consider adding proper error handling:
const alertChannelIds = (() => { try { return context.lookupCheckAlertChannels(resource.id) } catch (err) { + // If this is not a MissingContextVariableMappingError, rethrow + if (!(err instanceof MissingContextVariableMappingError)) { + throw err + } + // Otherwise, return undefined to indicate no alert channels } })()
112-119: Enhance error message for group lookup failure.The current error message when a check belongs to a group that is not being imported could be more descriptive to help with troubleshooting.
Provide a more informative error message:
try { const groupVariable = context.lookupCheckGroup(resource.groupId) builder.value('group', groupVariable) } catch (err) { - throw new Error('Check belongs to a group that is not being imported.') + throw new Error(`Check "${resource.name}" (ID: ${resource.id}) belongs to group ${resource.groupId} which is not being imported. Please include this group in your import selection.`) }packages/cli/src/constructs/incidentio-alert-channel-codegen.ts (2)
13-25: Consider adding a return type annotation to apiKeyFromHeaders.Adding a type annotation would improve TypeScript's ability to catch potential issues.
-function apiKeyFromHeaders (headers: HttpHeader[]): string | undefined { +function apiKeyFromHeaders (headers: HttpHeader[]): string | undefined {
49-54: Consider adding a dedicated function for API key extraction.The implementation extracts the API key from headers but embeds this logic directly within the
gencodemethod. Consider refactoring for better testability and reusability.if (config.headers) { const apiKey = apiKeyFromHeaders(config.headers) if (apiKey) { builder.string('apiKey', apiKey) } }The extraction logic is already in a dedicated function, which is good. Consider adding unit tests for the
apiKeyFromHeadersfunction to ensure it behaves as expected with different input formats.packages/cli/src/sourcegen/object.ts (3)
4-7: Optionalorderproperty
It might be helpful to add a docstring clarifying howorderis intended to be used if there's a default ordering mechanism.
38-51:sortObjectPropertiesByOrder
Provides a straightforward numeric sort. Consider clarifying that0is the default priority.
68-88:rendermethod
Nicely handles indentation and sorting. Consider whether trailing commas are acceptable in the generated output.packages/cli/src/constructs/construct-codegen.ts (3)
25-29:Resourceinterface
Properties are sufficiently generic. Potentially add doc comments describingpayload.
88-95:preparemethod usage
Unused parameterlogicalIdovershadowed byresource.logicalId. Consider removing or using the passed-inlogicalIdto avoid confusion.-prepare (logicalId: string, resource: Resource, context: Context): void { - codegen.prepare(resource.logicalId, resource.payload, context) +prepare (_: string, resource: Resource, context: Context): void { + codegen.prepare(resource.logicalId, resource.payload, context) }
97-104:gencodemethod
Same overshadowing oflogicalId. Consistency withpreparemethod changes is recommended.packages/cli/src/rest/projects.ts (3)
43-49:ImportPlaninterface
Adequately covers creation, apply, and commit timestamps. Consider clarifying howchangesis updated.
85-92:findImportPlansmethod
Filtering withonlyUnappliedandonlyUncommittedis a clear approach. Ensure pagination if the resource set grows large.
94-101:listImportPlansmethod
Global listing with optional filters is consistent withfindImportPlans. Possibly unify code for DRY principle.packages/cli/src/constructs/private-location-codegen.ts (1)
4-10: Consider adding doc comments to clarify usage.The
PrivateLocationResourceinterface is succinct and clear. However, adding JSDoc comments or TypeScript doc comments for each property can help other contributors understand the intended usage and constraints (especially forslugName,icon, andproxyUrl).packages/cli/src/constructs/check-group-codegen.ts (4)
45-92: Error swallowing in private location lookup.In lines 77-78, an error from
lookupCheckGroupPrivateLocationsis caught but silently ignored. Consider at least logging a warning to help diagnose missing context if the ID is invalid.
103-114: Check secret environment variables usage.The
environmentVariablesarray is appended without further logic to handle secrets. If secrets require special handling, consider clarifying or enforcing additional constraints here.
116-134: Similar fallback approach for alert channels.Like the private location logic, the code here also swallows errors when looking up alert channels. Evaluate whether silent failure is appropriate or if a warning/log statement can be beneficial.
180-229: Optional expansions for apiCheckDefaults.The property coverage is good, including headers, query parameters, and basic auth. While it appears sufficient, verify if advanced handlers (like OAuth flows) might need future expansions.
packages/cli/src/sourcegen/expr.ts (2)
42-57: Handle bracket notation for non-identifier properties.The comment suggests a future enhancement (line 55). Consider adding logic to decide whether to use dot or bracket notation, enabling usage with invalid identifier characters.
82-99: Potential expansion for more unary operators.At the moment, only
'!'is supported. Evaluate whether unary plus, minus, or bitwise operators are relevant for your use cases.packages/cli/src/constructs/alert-channel-codegen.ts (1)
21-29: Consider aligning optional checks with interface fields.All boolean fields are typed as required, yet the code checks for
!== undefinedinbuildAlertChannelProps. IfsendRecovery,sendFailure, etc., can indeed be omitted, mark them optional in the interface to avoid confusion.packages/cli/src/sourcegen/argsbuilder.ts (2)
19-62: Use tests to confirm value correctness
All the methods for building arguments appear well-structured. Ensure there is test coverage for each method to confirm that these different value types (strings, booleans, numbers, etc.) are emitted and rendered correctly.
64-76: Evaluate immutability or stricter usage
Consider whether mutating the same builder instance across multiple steps might introduce edge cases in complex codegen flows. An alternative pattern is returning a fresh builder when callingvalue()or marking the collection as read-only after building.packages/cli/src/constructs/internal/codegen/context.ts (1)
17-120: Validate potential re-registration cases
Each register method overwrites or adds new entries without verifying the existence of a previous mapping. If re-registering a resource with a different identifier is undesired, consider adding checks or warnings.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (65)
packages/cli/e2e/__tests__/help.spec.ts(2 hunks)packages/cli/package.json(1 hunks)packages/cli/src/commands/import/apply.ts(1 hunks)packages/cli/src/commands/import/cancel.ts(1 hunks)packages/cli/src/commands/import/commit.ts(1 hunks)packages/cli/src/commands/import/plan.ts(1 hunks)packages/cli/src/constructs/alert-channel-codegen.ts(1 hunks)packages/cli/src/constructs/alert-channel-subscription-codegen.ts(1 hunks)packages/cli/src/constructs/alert-escalation-policy-codegen.ts(1 hunks)packages/cli/src/constructs/api-check-codegen.ts(1 hunks)packages/cli/src/constructs/browser-check-codegen.ts(1 hunks)packages/cli/src/constructs/check-codegen.ts(1 hunks)packages/cli/src/constructs/check-group-codegen.ts(1 hunks)packages/cli/src/constructs/check-group.ts(1 hunks)packages/cli/src/constructs/construct-codegen.ts(1 hunks)packages/cli/src/constructs/dashboard-codegen.ts(1 hunks)packages/cli/src/constructs/email-alert-channel-codegen.ts(1 hunks)packages/cli/src/constructs/frequency-codegen.ts(1 hunks)packages/cli/src/constructs/heartbeat-check-codegen.ts(1 hunks)packages/cli/src/constructs/incidentio-alert-channel-codegen.ts(1 hunks)packages/cli/src/constructs/internal/assertion-codegen.ts(1 hunks)packages/cli/src/constructs/internal/codegen/codegen.ts(1 hunks)packages/cli/src/constructs/internal/codegen/context.ts(1 hunks)packages/cli/src/constructs/internal/codegen/index.ts(1 hunks)packages/cli/src/constructs/key-value-pair-codegen.ts(1 hunks)packages/cli/src/constructs/maintenance-window-codegen.ts(1 hunks)packages/cli/src/constructs/msteams-alert-channel-codegen.ts(1 hunks)packages/cli/src/constructs/multi-step-check-codegen.ts(1 hunks)packages/cli/src/constructs/opsgenie-alert-channel-codegen.ts(1 hunks)packages/cli/src/constructs/pagerduty-alert-channel-codegen.ts(1 hunks)packages/cli/src/constructs/phone-call-alert-channel-codegen.ts(1 hunks)packages/cli/src/constructs/playwright-config-codegen.ts(1 hunks)packages/cli/src/constructs/playwright-config.ts(0 hunks)packages/cli/src/constructs/private-location-check-assignment-codegen.ts(1 hunks)packages/cli/src/constructs/private-location-codegen.ts(1 hunks)packages/cli/src/constructs/private-location-group-assignment-codegen.ts(1 hunks)packages/cli/src/constructs/ref-codegen.ts(1 hunks)packages/cli/src/constructs/retry-strategy-codegen.ts(1 hunks)packages/cli/src/constructs/slack-alert-channel-codegen.ts(1 hunks)packages/cli/src/constructs/sms-alert-channel-codegen.ts(1 hunks)packages/cli/src/constructs/tcp-check-codegen.ts(1 hunks)packages/cli/src/constructs/telegram-alert-channel-codegen.ts(1 hunks)packages/cli/src/constructs/webhook-alert-channel-codegen.ts(1 hunks)packages/cli/src/rest/projects.ts(3 hunks)packages/cli/src/sourcegen/args.ts(1 hunks)packages/cli/src/sourcegen/argsbuilder.ts(1 hunks)packages/cli/src/sourcegen/array.ts(1 hunks)packages/cli/src/sourcegen/arraybuilder.ts(1 hunks)packages/cli/src/sourcegen/boolean.ts(1 hunks)packages/cli/src/sourcegen/decl.ts(1 hunks)packages/cli/src/sourcegen/declbuilder.ts(1 hunks)packages/cli/src/sourcegen/expr.ts(1 hunks)packages/cli/src/sourcegen/exprbuilder.ts(1 hunks)packages/cli/src/sourcegen/identifier.ts(1 hunks)packages/cli/src/sourcegen/index.ts(1 hunks)packages/cli/src/sourcegen/null.ts(1 hunks)packages/cli/src/sourcegen/number.ts(1 hunks)packages/cli/src/sourcegen/object.ts(1 hunks)packages/cli/src/sourcegen/objectbuilder.ts(1 hunks)packages/cli/src/sourcegen/output.ts(1 hunks)packages/cli/src/sourcegen/program.ts(1 hunks)packages/cli/src/sourcegen/string.ts(1 hunks)packages/cli/src/sourcegen/undefined.ts(1 hunks)packages/cli/src/sourcegen/unknown.ts(1 hunks)packages/cli/src/sourcegen/value.ts(1 hunks)
💤 Files with no reviewable changes (1)
- packages/cli/src/constructs/playwright-config.ts
🚧 Files skipped from review as they are similar to previous changes (36)
- packages/cli/src/sourcegen/undefined.ts
- packages/cli/src/sourcegen/null.ts
- packages/cli/src/sourcegen/boolean.ts
- packages/cli/e2e/tests/help.spec.ts
- packages/cli/src/sourcegen/number.ts
- packages/cli/src/constructs/internal/codegen/index.ts
- packages/cli/src/sourcegen/value.ts
- packages/cli/package.json
- packages/cli/src/sourcegen/program.ts
- packages/cli/src/sourcegen/args.ts
- packages/cli/src/constructs/playwright-config-codegen.ts
- packages/cli/src/constructs/check-group.ts
- packages/cli/src/constructs/browser-check-codegen.ts
- packages/cli/src/sourcegen/identifier.ts
- packages/cli/src/commands/import/cancel.ts
- packages/cli/src/sourcegen/array.ts
- packages/cli/src/sourcegen/unknown.ts
- packages/cli/src/sourcegen/string.ts
- packages/cli/src/constructs/ref-codegen.ts
- packages/cli/src/constructs/frequency-codegen.ts
- packages/cli/src/commands/import/commit.ts
- packages/cli/src/constructs/private-location-check-assignment-codegen.ts
- packages/cli/src/constructs/multi-step-check-codegen.ts
- packages/cli/src/constructs/internal/codegen/codegen.ts
- packages/cli/src/constructs/retry-strategy-codegen.ts
- packages/cli/src/constructs/key-value-pair-codegen.ts
- packages/cli/src/constructs/dashboard-codegen.ts
- packages/cli/src/sourcegen/output.ts
- packages/cli/src/constructs/telegram-alert-channel-codegen.ts
- packages/cli/src/constructs/maintenance-window-codegen.ts
- packages/cli/src/constructs/alert-escalation-policy-codegen.ts
- packages/cli/src/sourcegen/exprbuilder.ts
- packages/cli/src/constructs/heartbeat-check-codegen.ts
- packages/cli/src/sourcegen/decl.ts
- packages/cli/src/sourcegen/index.ts
- packages/cli/src/constructs/alert-channel-subscription-codegen.ts
🧰 Additional context used
🧬 Code Definitions (17)
packages/cli/src/constructs/opsgenie-alert-channel-codegen.ts (3)
packages/cli/src/constructs/alert-channel-codegen.ts (2)
AlertChannelResource(21-29)buildAlertChannelProps(31-51)packages/cli/src/constructs/internal/codegen/index.ts (2)
Codegen(1-1)Context(2-2)packages/cli/src/constructs/internal/codegen/context.ts (1)
Context(5-121)
packages/cli/src/constructs/api-check-codegen.ts (5)
packages/cli/src/constructs/check-codegen.ts (2)
CheckResource(17-35)buildCheckProps(37-154)packages/cli/src/constructs/internal/assertion-codegen.ts (2)
valueForNumericAssertion(4-43)valueForGeneralAssertion(45-146)packages/cli/src/constructs/internal/codegen/index.ts (2)
Codegen(1-1)Context(2-2)packages/cli/src/constructs/internal/codegen/context.ts (1)
Context(5-121)packages/cli/src/constructs/key-value-pair-codegen.ts (1)
valueForKeyValuePair(4-17)
packages/cli/src/constructs/private-location-group-assignment-codegen.ts (2)
packages/cli/src/constructs/internal/codegen/index.ts (2)
Codegen(1-1)Context(2-2)packages/cli/src/constructs/internal/codegen/context.ts (1)
Context(5-121)
packages/cli/src/constructs/email-alert-channel-codegen.ts (3)
packages/cli/src/constructs/alert-channel-codegen.ts (2)
AlertChannelResource(21-29)buildAlertChannelProps(31-51)packages/cli/src/constructs/internal/codegen/index.ts (2)
Codegen(1-1)Context(2-2)packages/cli/src/constructs/internal/codegen/context.ts (1)
Context(5-121)
packages/cli/src/constructs/sms-alert-channel-codegen.ts (2)
packages/cli/src/constructs/alert-channel-codegen.ts (2)
AlertChannelResource(21-29)buildAlertChannelProps(31-51)packages/cli/src/constructs/internal/codegen/context.ts (1)
Context(5-121)
packages/cli/src/constructs/private-location-codegen.ts (3)
packages/cli/src/sourcegen/program.ts (1)
Program(7-53)packages/cli/src/constructs/internal/codegen/index.ts (2)
Codegen(1-1)Context(2-2)packages/cli/src/constructs/internal/codegen/context.ts (1)
Context(5-121)
packages/cli/src/constructs/webhook-alert-channel-codegen.ts (7)
packages/cli/src/constructs/alert-channel-codegen.ts (2)
AlertChannelResource(21-29)buildAlertChannelProps(31-51)packages/cli/src/constructs/key-value-pair-codegen.ts (1)
valueForKeyValuePair(4-17)packages/cli/src/constructs/internal/codegen/index.ts (2)
Codegen(1-1)Context(2-2)packages/cli/src/constructs/incidentio-alert-channel-codegen.ts (1)
IncidentioAlertChannelCodegen(29-62)packages/cli/src/constructs/msteams-alert-channel-codegen.ts (1)
MSTeamsAlertChannelCodegen(14-40)packages/cli/src/constructs/telegram-alert-channel-codegen.ts (1)
TelegramAlertChannelCodegen(28-67)packages/cli/src/constructs/internal/codegen/context.ts (1)
Context(5-121)
packages/cli/src/constructs/incidentio-alert-channel-codegen.ts (3)
packages/cli/src/constructs/webhook-alert-channel-codegen.ts (2)
WebhookAlertChannelResource(28-31)WebhookAlertChannelResourceConfig(17-26)packages/cli/src/constructs/internal/codegen/context.ts (1)
Context(5-121)packages/cli/src/constructs/alert-channel-codegen.ts (1)
buildAlertChannelProps(31-51)
packages/cli/src/constructs/tcp-check-codegen.ts (5)
packages/cli/src/constructs/check-codegen.ts (2)
CheckResource(17-35)buildCheckProps(37-154)packages/cli/src/sourcegen/program.ts (1)
Program(7-53)packages/cli/src/constructs/internal/assertion-codegen.ts (2)
valueForGeneralAssertion(45-146)valueForNumericAssertion(4-43)packages/cli/src/constructs/internal/codegen/index.ts (2)
Codegen(1-1)Context(2-2)packages/cli/src/constructs/internal/codegen/context.ts (1)
Context(5-121)
packages/cli/src/sourcegen/object.ts (3)
packages/cli/src/sourcegen/index.ts (3)
Value(33-33)ObjectValue(26-26)Output(28-28)packages/cli/src/sourcegen/objectbuilder.ts (1)
value(67-70)packages/cli/src/sourcegen/output.ts (1)
Output(40-85)
packages/cli/src/constructs/phone-call-alert-channel-codegen.ts (3)
packages/cli/src/constructs/alert-channel-codegen.ts (2)
AlertChannelResource(21-29)buildAlertChannelProps(31-51)packages/cli/src/constructs/internal/codegen/index.ts (2)
Codegen(1-1)Context(2-2)packages/cli/src/constructs/internal/codegen/context.ts (1)
Context(5-121)
packages/cli/src/constructs/construct-codegen.ts (12)
packages/cli/src/constructs/internal/codegen/index.ts (2)
Codegen(1-1)Context(2-2)packages/cli/src/constructs/alert-channel-codegen.ts (1)
AlertChannelCodegen(66-114)packages/cli/src/constructs/alert-channel-subscription-codegen.ts (1)
AlertChannelSubscriptionCodegen(9-24)packages/cli/src/constructs/check-codegen.ts (1)
CheckCodegen(156-192)packages/cli/src/constructs/check-group-codegen.ts (1)
CheckGroupCodegen(241-262)packages/cli/src/constructs/dashboard-codegen.ts (1)
DashboardCodegen(31-140)packages/cli/src/constructs/maintenance-window-codegen.ts (1)
MaintenanceWindowCodegen(16-64)packages/cli/src/constructs/private-location-codegen.ts (1)
PrivateLocationCodegen(25-55)packages/cli/src/constructs/private-location-check-assignment-codegen.ts (1)
PrivateLocationCheckAssignmentCodegen(8-17)packages/cli/src/constructs/private-location-group-assignment-codegen.ts (1)
PrivateLocationGroupAssignmentCodegen(8-17)packages/cli/src/sourcegen/program.ts (1)
Program(7-53)packages/cli/src/constructs/internal/codegen/context.ts (1)
Context(5-121)
packages/cli/src/constructs/check-codegen.ts (14)
packages/cli/src/constructs/frequency-codegen.ts (2)
FrequencyResource(4-4)valueForFrequency(6-54)packages/cli/src/constructs/alert-escalation-policy-codegen.ts (2)
AlertEscalationResource(4-4)valueForAlertEscalation(6-65)packages/cli/src/constructs/retry-strategy-codegen.ts (2)
RetryStrategyResource(4-4)valueForRetryStrategy(6-77)packages/cli/src/sourcegen/objectbuilder.ts (1)
ObjectValueBuilder(18-75)packages/cli/src/constructs/internal/codegen/index.ts (2)
Context(2-2)Codegen(1-1)packages/cli/src/constructs/internal/codegen/context.ts (1)
Context(5-121)packages/cli/src/constructs/private-location-codegen.ts (1)
valueForPrivateLocationFromId(14-23)packages/cli/src/constructs/key-value-pair-codegen.ts (1)
valueForKeyValuePair(4-17)packages/cli/src/constructs/alert-channel-codegen.ts (1)
valueForAlertChannelFromId(55-64)packages/cli/src/constructs/api-check-codegen.ts (2)
ApiCheckCodegen(42-153)ApiCheckResource(8-19)packages/cli/src/constructs/check-group-codegen.ts (1)
CheckGroupCodegen(241-262)packages/cli/src/constructs/heartbeat-check-codegen.ts (2)
HeartbeatCheckCodegen(11-29)HeartbeatCheckResource(6-9)packages/cli/src/constructs/multi-step-check-codegen.ts (2)
MultiStepCheckCodegen(15-42)MultiStepCheckResource(6-11)packages/cli/src/constructs/tcp-check-codegen.ts (2)
TcpCheckCodegen(29-72)TcpCheckResource(7-12)
packages/cli/src/constructs/alert-channel-codegen.ts (9)
packages/cli/src/sourcegen/program.ts (1)
Program(7-53)packages/cli/src/constructs/internal/codegen/index.ts (2)
Codegen(1-1)Context(2-2)packages/cli/src/constructs/phone-call-alert-channel-codegen.ts (1)
PhoneCallAlertChannelCodegen(15-44)packages/cli/src/constructs/email-alert-channel-codegen.ts (1)
EmailAlertChannelCodegen(14-37)packages/cli/src/constructs/opsgenie-alert-channel-codegen.ts (1)
OpsgenieAlertChannelCodegen(17-45)packages/cli/src/constructs/pagerduty-alert-channel-codegen.ts (1)
PagerdutyAlertChannelCodegen(16-49)packages/cli/src/constructs/slack-alert-channel-codegen.ts (1)
SlackAlertChannelCodegen(15-44)packages/cli/src/constructs/sms-alert-channel-codegen.ts (1)
SmsAlertChannelCodegen(15-44)packages/cli/src/constructs/webhook-alert-channel-codegen.ts (1)
WebhookAlertChannelCodegen(78-136)
packages/cli/src/constructs/check-group-codegen.ts (9)
packages/cli/src/constructs/frequency-codegen.ts (2)
FrequencyResource(4-4)valueForFrequency(6-54)packages/cli/src/constructs/alert-escalation-policy-codegen.ts (2)
AlertEscalationResource(4-4)valueForAlertEscalation(6-65)packages/cli/src/constructs/retry-strategy-codegen.ts (2)
RetryStrategyResource(4-4)valueForRetryStrategy(6-77)packages/cli/src/constructs/internal/codegen/index.ts (2)
Context(2-2)Codegen(1-1)packages/cli/src/constructs/internal/codegen/context.ts (1)
Context(5-121)packages/cli/src/constructs/private-location-codegen.ts (1)
valueForPrivateLocationFromId(14-23)packages/cli/src/constructs/key-value-pair-codegen.ts (1)
valueForKeyValuePair(4-17)packages/cli/src/constructs/alert-channel-codegen.ts (1)
valueForAlertChannelFromId(55-64)packages/cli/src/constructs/api-check-codegen.ts (1)
valueForAssertion(21-38)
packages/cli/src/sourcegen/argsbuilder.ts (2)
packages/cli/src/sourcegen/exprbuilder.ts (4)
build(39-41)context(34-37)ExpressionValueBuilder(11-42)expr(5-9)packages/cli/src/sourcegen/args.ts (1)
ArgumentsValue(4-27)
packages/cli/src/constructs/msteams-alert-channel-codegen.ts (4)
packages/cli/src/constructs/webhook-alert-channel-codegen.ts (2)
WebhookAlertChannelResource(28-31)WebhookAlertChannelResourceConfig(17-26)packages/cli/src/constructs/internal/codegen/index.ts (2)
Codegen(1-1)Context(2-2)packages/cli/src/constructs/internal/codegen/context.ts (1)
Context(5-121)packages/cli/src/constructs/alert-channel-codegen.ts (1)
buildAlertChannelProps(31-51)
🪛 Biome (1.9.4)
packages/cli/src/constructs/check-codegen.ts
[error] 179-179: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
[error] 181-181: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
[error] 183-183: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
[error] 185-185: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
[error] 187-187: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
🔇 Additional comments (61)
packages/cli/src/constructs/internal/assertion-codegen.ts (2)
15-38: Potential float precision issue when parsing numbers
This section usesparseIntfor numeric comparisons, which may cause precision issues if floating-point values are involved. A previous review comment suggested usingNumber(...)instead ofparseInt.- builder.number(parseInt(assertion.target, 10)) + builder.number(Number(assertion.target))
106-117: String-based numeric comparisons could cause unexpected results
Performing'LESS_THAN'and'GREATER_THAN'checks with string inputs can be incorrect for numeric comparisons. This was raised in a previous review. Consider dynamically checking if the target is numeric first and using.number(...)when appropriate.case 'LESS_THAN': builder.member(ident('lessThan')) builder.call(builder => { - builder.string(assertion.target) + const numValue = Number(assertion.target) + if (!isNaN(numValue)) { + builder.number(numValue) + } else { + builder.string(assertion.target) + } }) breakpackages/cli/src/constructs/api-check-codegen.ts (1)
21-38: Straightforward assertion handling
ThevalueForAssertionfunction consistently delegates numeric and general assertions to the respective handlers. The error handling for unsupported sources appears robust. Nice work.packages/cli/src/commands/import/apply.ts (1)
62-95: Plan Selection Logic Appears Robust.The method correctly maps plans to distinct choices, handles cancellation scenarios, and exits cleanly if the user selects "Exit." No further concerns.
packages/cli/src/constructs/slack-alert-channel-codegen.ts (1)
15-45: Implementation Successfully Integrates Slack Alert Channel.The code correctly uses the context to register the alert channel and generate the Slack construct. Looks good overall.
packages/cli/src/constructs/email-alert-channel-codegen.ts (1)
14-36: Code Generation Logic for Email Alert Channel Looks Solid.Registration in
Contextand subsequent generation withbuildAlertChannelPropsis implemented cleanly. No issues found here.packages/cli/src/commands/import/plan.ts (1)
29-51: [Duplicate of previous feedback] Add unit tests for existing plan handling.A prior review comment already highlighted insufficient test coverage regarding existing import plans. Specifically, lines 29–51 should be tested to ensure the workflow is correct when existing plans are found. Please add these tests or confirm they already exist to address past feedback.
packages/cli/src/constructs/tcp-check-codegen.ts (3)
7-12: Confirm property naming and data types inTcpCheckResource.The fields
degradedResponseTimeandmaxResponseTimeare optional numeric values. Ensure consuming code (e.g., resource import or plan application) uses consistent naming and handles undefined values gracefully.
14-25: Good use of a default case for unknown assertion sources.The
defaultbranch invalueForTcpAssertionappropriately throws an error for unsupported sources, preventing silent failures.
29-71: Overall implementation is clear and maintainable.The class effectively imports the
TcpCheckconstruct, sets up resource requests, and extends the base code generation approach. No critical issues noted.packages/cli/src/constructs/sms-alert-channel-codegen.ts (1)
16-19: Consider verifying test coverage forpreparemethod.This method registers the alert channel in the context. Be sure to add or confirm the presence of tests that verify the mapping is set correctly in various scenarios (e.g., multiple SMS channels, duplicates).
packages/cli/src/sourcegen/declbuilder.ts (2)
1-3: Imports look consistent with usage.
All imported entities (Declaration,IdentifierDeclaration, etc.) are properly utilized and help maintain strong typing.
5-9: Builder pattern is straightforward and adaptable.
Thedeclfunction succinctly captures and applies the builder logic, keeping the implementation simple and readable.packages/cli/src/constructs/msteams-alert-channel-codegen.ts (2)
6-10: Interface properly extends the webhook resource.
Ensures theMSTeamsAlertChannelResourceinherits all relevant webhook fields while enforcing a fixedwebhookType.
15-17: Alert channel registration aligns with existing context usage.
Registering the channel with ateamsAlertprefix is consistent with naming strategies for other alert types.packages/cli/src/constructs/phone-call-alert-channel-codegen.ts (2)
5-11: Resource interface is clear and minimal.
Defines only the necessary properties (type,config) to represent a phone call alert channel.
16-18: Intentional naming for channel prefix.
Registering the alert channel with'phoneCallAlert'ensures consistent variable naming across the codebase.packages/cli/src/constructs/check-codegen.ts (1)
174-191: Remove "return" statements in a void function.The
gencodemethod is declared with a return type ofvoid, but it's returning values from the function calls in the switch statement. This is flagged by Biome as a correctness issue.Apply this diff to fix the issues:
gencode (logicalId: string, resource: CheckResource, context: Context): void { const { checkType } = resource switch (checkType) { case 'BROWSER': - return this.browserCheckCodegen.gencode(logicalId, resource as BrowserCheckResource, context) + this.browserCheckCodegen.gencode(logicalId, resource as BrowserCheckResource, context) + break case 'API': - return this.apiCheckCodegen.gencode(logicalId, resource as ApiCheckResource, context) + this.apiCheckCodegen.gencode(logicalId, resource as ApiCheckResource, context) + break case 'TCP': - return this.tcpCheckCodegen.gencode(logicalId, resource as TcpCheckResource, context) + this.tcpCheckCodegen.gencode(logicalId, resource as TcpCheckResource, context) + break case 'MULTI_STEP': - return this.multiStepCheckCodegen.gencode(logicalId, resource as MultiStepCheckResource, context) + this.multiStepCheckCodegen.gencode(logicalId, resource as MultiStepCheckResource, context) + break case 'HEARTBEAT': - return this.heartbeatCheckCodegen.gencode(logicalId, resource as HeartbeatCheckResource, context) + this.heartbeatCheckCodegen.gencode(logicalId, resource as HeartbeatCheckResource, context) + break default: throw new Error(`Unable to generate code for unsupported check type '${checkType}'.`) } }🧰 Tools
🪛 Biome (1.9.4)
[error] 179-179: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
[error] 181-181: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
[error] 183-183: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
[error] 185-185: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
[error] 187-187: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
packages/cli/src/constructs/opsgenie-alert-channel-codegen.ts (1)
1-45: LGTM! Well-structured OpsGenie alert channel codegen implementation.The implementation is clean and follows the established pattern for alert channel codegen classes, with proper use of the builder pattern and error handling.
packages/cli/src/constructs/pagerduty-alert-channel-codegen.ts (1)
1-49: LGTM! Well-structured PagerDuty alert channel codegen implementation.The implementation correctly handles optional fields with conditional inclusion, follows the established pattern for alert channel codegen classes, and properly uses the builder pattern.
packages/cli/src/sourcegen/object.ts (8)
1-3: Imports look good
Straightforward imports from local modules. No issues spotted.
8-18: ClassObjectPropertyis well-structured
All fields and constructor usage look clean.
20-21: Sorter type
Defining a dedicated type for the sorter enhances readability and maintainability.
22-24: Optional sorter inObjectValueOptions
Having a configurable sorter is a nice design for flexible property ordering.
26-36:sortObjectPropertiesByName
Clear, simple string comparison. No issues found.
53-56:sortObjectPropertiesByOrderAndName
Combines sorting logic well. Implementation is concise and correct.
58-61:ObjectValueextendsValue
Good extension approach. The additional properties and options are well-defined here.
62-66:ObjectValueconstructor
Successfully delegates setup tosuper()and sets optional fields. No concerns.packages/cli/src/constructs/construct-codegen.ts (4)
14-24: Introduction ofResourceType
Enumerating resource types in a string union helps clarify valid resource kinds.
43-49:sortResourcesfunction
Descending sort logic is correct (return bo - ao).
51-86:ConstructCodegenclass fields
Declaring multiple codegen fields is tidy. The map-based approach incodegensByTypeis easy to maintain.
31-41:✅ Verification successful
resourceOrderobject
Straightforward numeric ordering. Confirm that no resource type is missing.
🏁 Script executed:
#!/bin/bash # Check if all ResourceTypes are in the resourceOrder object rg "type ResourceType" -A 10 rg "const resourceOrder" -A 20Length of output: 2471
ResourceOrder Definition Verified
The verification confirms that every resource type declared in theResourceTypeunion is correctly represented in theresourceOrderobject. No resource type is missing, and the numeric ordering appears complete and consistent.packages/cli/src/rest/projects.ts (6)
27-27:ResourceSyncinterface
No immediate issues. Ensure thatmemberis consistently used.
39-41:ImportPlanChangesinterface
Defines an array ofResourceSyncobjects. Straightforward structure.
103-105:cancelImportPlanmethod
Straightforward DELETE request. Check that partial cancellations are handled or if there's a progress lock.
107-109:applyImportPlanmethod
Applies an import plan. Consider verifying if a plan can only be applied once.
111-113:commitImportPlanmethod
Finish the import plan lifecycle. Good separation of responsibilities.
81-83:❓ Verification inconclusive
createImportPlanmethod
POST request to create a plan. Please confirm error handling in case of invalid logicalId.
🏁 Script executed:
#!/bin/bash # Searching for error-handling references in createImportPlan usage rg "createImportPlan"Length of output: 207
Attention: Confirm Error Handling for Invalid LogicalId in Import Plan Creation
The
createImportPlanmethod (inpackages/cli/src/rest/projects.ts) simply wraps the POST request and does not itself implement explicit error handling. The only usage we found—inpackages/cli/src/commands/import/plan.ts—calls the method directly without a surrounding try/catch block. This indicates that error handling for an invalidlogicalIdis likely expected to be managed upstream or via a global error handler.
- Verify that the calling code or global middleware properly catches and handles errors from this API call.
- If not already in place, consider adding explicit error handling (e.g., try/catch or promise rejection handling) around the call in
packages/cli/src/commands/import/plan.ts.packages/cli/src/constructs/private-location-codegen.ts (3)
14-23: Ensure error handling for invalid IDs.
valueForPrivateLocationFromIdsimply invokesPrivateLocation.fromIdwithout handling potential errors if the physical ID is invalid. Confirm whether your codebase expects such an exception upstream or if you should handle it more gracefully.
25-29: Looks good!The
preparemethod properly registers the private location. This approach is consistent with the rest of the context registration pattern.
30-54: Implementation is solid.Creating a new instance of
PrivateLocationconditionally includes optional parameters, maintaining a clear and straightforward code generation flow.packages/cli/src/constructs/check-group-codegen.ts (6)
13-22: Browser and multi-step check interfaces look good.
BrowserCheckConfigResourceandMultiStepCheckConfigResourcewell-encapsulate their respective fields. Future expansions (e.g., advanced configuration) can be easily added.
23-43: Review required for concurrency and runParallel parameters.The
CheckGroupResourceinterface containsconcurrencyandrunParallelfields separately, which might overlap in meaning. Validate if both should coexist or if they can be unified to prevent confusion.
136-158: Validate additional browser check fields beyond frequency.Currently, only
testMatchandfrequencyare handled for browser checks. Confirm if other fields (e.g.,runtimeId, timeouts) could be relevant and missed here.
160-178: Multi-step checks logic parallels browser checks nicely.No immediate issues. The approach reuses the same pattern to handle
testMatchandfrequency, providing consistency across check types.
232-236: Default retry strategy properly conveyed.
valueForRetryStrategycovers multiple strategies, ensuring a safe default of no retries. This is a clean way to preserve extension capability.
239-262: Class design is consistent.The
prepareandgencodemethods parallel the rest of the codegen modules, promoting clarity and uniformity across resource types.packages/cli/src/sourcegen/expr.ts (4)
5-7: Abstract class is appropriately minimal.
ExpressionValuesets the foundation for subclass expression types. The current design is straightforward and flexible.
8-24: Constructor-based approach is clean.
NewExpressionValuestraightforwardly manages the 'new' keyword, keeping the code maintainable and easily extensible.
26-40: No improvements needed.
CallExpressionValuelogically captures function calls. Ensuring argument rendering is consistent with the rest of the expression model.
59-80: Limited operator support.Currently only
+,-,&&, and||are supported inBinaryOperator. If you need other operators (e.g.,*,/, comparison operators), extending this type is straightforward.packages/cli/src/constructs/alert-channel-codegen.ts (3)
53-64: FunctionvalueForAlertChannelFromIdis well-designed.This utility neatly encapsulates the logic for retrieving an alert channel from its physical ID, and the import ensures that the relevant construct is used. No issues found.
66-95: Constructor logic with typed codegens is clear and organized.The record
codegensByTypecleanly maps eachAlertChannelTypeto a specialized codegen instance. This pattern is easy to extend for future alert channel types.
97-114:prepareandgencodemethods handle unsupported types gracefully.Throwing an error on an unknown
resource.typeis an effective safeguard. The forward calls to the correct specialized codegen are well-structured.packages/cli/src/constructs/webhook-alert-channel-codegen.ts (2)
79-80: Typographical mismatch inindicentioCodegenvs.IncidentioAlertChannelCodegen.This was raised before. Consider renaming the property to align with the class name (“incidentio”) for clarity.
97-135: Granular delegation inprepareandgencodeis consistent.The conditional logic properly routes each webhook type to its respective specialized codegen. Registering the alert channel in context if no specialized codegen is found is a sensible fallback.
packages/cli/src/sourcegen/arraybuilder.ts (1)
1-75: Array builder API is comprehensive and well-structured.Each method (e.g.
string,boolean,ident) is intuitive, ensuring readability and extensibility. No concerns found with performance or correctness.packages/cli/src/sourcegen/argsbuilder.ts (1)
13-17: Implementation is straightforward
No issues found with theargsfunction. The approach of delegating to the builder is clean and consistent with typical builder patterns.packages/cli/src/sourcegen/objectbuilder.ts (2)
12-16: Straightforward object builder entrypoint
The top-levelobjectfunction nicely encapsulates the builder usage. This pattern is consistent with your approach inargsbuilder.ts.
18-76: Consider detection of duplicate property names
There is no check for accidentally adding the same property multiple times. If silent overwriting is unintentional, consider adding a guard or logging to alert developers when a property is already present.packages/cli/src/constructs/internal/codegen/context.ts (1)
5-15: Good use of private maps
Storing context variables in private maps helps encapsulate resource-specific tracking and prevents direct external mutation. No immediate concerns here.
| builder.string(logicalId) | ||
| builder.object(builder => { | ||
| builder.string('name', config.name) | ||
| builder.string('url', config.url.toString()) |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Check if config.url needs toString() method.
Based on the interface, config.url should already be a string, but toString() is being called on it.
🏁 Script executed:
#!/bin/bash
# Check whether url is a string or an object in WebhookAlertChannelResourceConfig
echo "Looking for WebhookAlertChannelResourceConfig definition..."
rg -A 20 "interface WebhookAlertChannelResourceConfig" --type ts
echo "Looking for url usage in other alert channel codegen files..."
rg "config\.url\.toString\(\)" --type tsLength of output: 2429
Remove the redundant .toString() call on config.url
- The interface for
WebhookAlertChannelResourceConfigdefinesurlas a string. - Since
config.urlis already a string, calling.toString()is unnecessary. - Please remove
.toString()frombuilder.string('url', config.url.toString())for clarity and consistency in bothincidentio-alert-channel-codegen.tsand similar usages in the codebase.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/cli/src/constructs/webhook-alert-channel-codegen.ts (1)
33-74: Consider removing redundanttoString()call.
config.urlis already astring, so callingtoString()is not strictly necessary. You can safely omit it or convert it only if some special string formatting is required.- builder.string('url', config.url.toString()) + builder.string('url', config.url)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/cli/src/constructs/webhook-alert-channel-codegen.ts(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
packages/cli/src/constructs/webhook-alert-channel-codegen.ts (7)
packages/cli/src/constructs/alert-channel-codegen.ts (2)
AlertChannelResource(21-29)buildAlertChannelProps(31-51)packages/cli/src/constructs/key-value-pair-codegen.ts (1)
valueForKeyValuePair(4-17)packages/cli/src/constructs/internal/codegen/index.ts (2)
Codegen(1-1)Context(2-2)packages/cli/src/constructs/incidentio-alert-channel-codegen.ts (1)
IncidentioAlertChannelCodegen(29-62)packages/cli/src/constructs/msteams-alert-channel-codegen.ts (1)
MSTeamsAlertChannelCodegen(14-40)packages/cli/src/constructs/telegram-alert-channel-codegen.ts (1)
TelegramAlertChannelCodegen(28-67)packages/cli/src/constructs/internal/codegen/context.ts (1)
Context(5-121)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: test - windows-latest
- GitHub Check: test - ubuntu-latest
🔇 Additional comments (5)
packages/cli/src/constructs/webhook-alert-channel-codegen.ts (5)
1-16: Imports and WebhookType look consistent.
These imports and theWebhookTypedefinition align well with the other alert channel codegens. No issues found here.
17-31: OptionalwebhookTypeusage is clear.
MakingwebhookTypeoptional provides flexibility for fallback generation whenwebhookTypeis not specified.
76-95: Codegen constructors look valid.
The constructor properly initializes subtype codegens and keeps a neat mapping incodegensByWebhookType.
97-108: No concerns with theprepareimplementation.
The fallback path of registering a generic “webhookAlert” is a good approach for unrecognized or missingwebhookType.
110-135: Request tests for fallback scenario.
WhenwebhookTypeis missing or unrecognized, the code uses the fallback path. Consider adding a dedicated test to ensure the resulting resource is generated as expected.Would you like me to generate a test to verify the fallback generation scenario?
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
packages/cli/src/constructs/api-check-codegen.ts (2)
118-132: Consider refactoring duplicated script handling logicThe setup and teardown script handling code is nearly identical, with only naming differences. This could be extracted into a helper method to avoid code duplication.
+ private buildScriptContent( + builder: any, + propertyName: string, + localScript?: string, + scriptResource?: SnippetResource + ): void { + if (localScript) { + const content = localScript; + builder.object(propertyName, builder => { + builder.string('content', content); + }); + } else if (scriptResource?.script) { + const script = scriptResource.script; + builder.object(propertyName, builder => { + // TODO: Move to a separate file and use entrypoint instead. + builder.string('content', script); + }); + } + } gencode (logicalId: string, resource: ApiCheckResource, context: Context): void { // ... existing code ... - if (resource.localSetupScript) { - const content = resource.localSetupScript - builder.object('setupScript', builder => { - builder.string('content', content) - }) - } else if (resource.setupScript) { - const snippet = resource.setupScript - if (snippet.script !== undefined) { - const script = snippet.script - builder.object('setupScript', builder => { - // TODO: Move to a separate file and use entrypoint instead. - builder.string('content', script) - }) - } - } + this.buildScriptContent( + builder, + 'setupScript', + resource.localSetupScript, + resource.setupScript + ); - if (resource.localTearDownScript) { - const content = resource.localTearDownScript - builder.object('tearDownScript', builder => { - builder.string('content', content) - }) - } else if (resource.tearDownScript) { - const snippet = resource.tearDownScript - if (snippet.script !== undefined) { - const script = snippet.script - builder.object('tearDownScript', builder => { - // TODO: Move to a separate file and use entrypoint instead. - builder.string('content', script) - }) - } - } + this.buildScriptContent( + builder, + 'tearDownScript', + resource.localTearDownScript, + resource.tearDownScript + );Also applies to: 134-148
108-115: Validate empty strings in basicAuth checkThe code checks if the username and password are not empty strings before including basicAuth. Consider using a more robust validation that also checks for undefined/null values.
- if (basicAuth.username !== '' && basicAuth.password !== '') { + if (basicAuth.username && basicAuth.password) {packages/cli/src/constructs/private-location-codegen.ts (2)
4-10: Well-defined interface with appropriate optional propertiesThe
PrivateLocationResourceinterface clearly defines the structure with required fields (id,name,slugName) and optional fields (icon,proxyUrl) properly marked with the optional operator?.Consider adding JSDoc comments to document the purpose of this interface and its properties for better developer experience.
30-56: Code generation method with proper structure and conditional propertiesThe
gencodemethod:
- Imports the necessary construct
- Looks up the previously registered ID
- Generates appropriate code with conditional properties
- Properly exports the generated variable
The conditional inclusion of optional properties (
iconandproxyUrl) only when they are provided is a good practice.I would recommend adding error handling for the context lookup operation, as it will throw if the resource ID is not found.
gencode (logicalId: string, resource: PrivateLocationResource, context: Context): void { this.program.import(construct, 'checkly/constructs') - const id = context.lookupPrivateLocation(resource.id) + let id; + try { + id = context.lookupPrivateLocation(resource.id) + } catch (error) { + throw new Error(`Failed to lookup private location with ID ${resource.id}: ${error.message}`) + } this.program.section(decl(id, builder => {packages/cli/src/constructs/check-group-codegen.ts (1)
45-237: Consider splittingbuildCheckGroupPropsinto smaller, focused functions.The current function spans nearly 200 lines, handling many conditionals and sub-structures. Breaking it into smaller helper functions (e.g.,
populateLocations,populatePrivateLocations,populateAlertChannels, etc.) would improve readability and maintainability.packages/cli/src/constructs/sms-alert-channel-codegen.ts (2)
5-11: Property naming inconsistency between interface and implementation.The interface defines the SMS number property as
numberin the config, but in the implementation it's referred to asphoneNumber(line 36). This naming inconsistency could cause confusion.Consider making the property names consistent:
export interface SmsAlertChannelResource extends AlertChannelResource { type: 'SMS' config: { - number: string + phoneNumber: string name?: string } }
36-36: Property names should match interface definition.This line uses
phoneNumberbut the interface defines it asnumber.- builder.string('phoneNumber', config.number) + builder.string('phoneNumber', config.phoneNumber)Alternatively, if you want to keep the interface as is:
- builder.string('phoneNumber', config.number) + builder.string('phoneNumber', config.number)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
packages/cli/src/constructs/api-check-codegen.ts(1 hunks)packages/cli/src/constructs/check-group-codegen.ts(1 hunks)packages/cli/src/constructs/email-alert-channel-codegen.ts(1 hunks)packages/cli/src/constructs/incidentio-alert-channel-codegen.ts(1 hunks)packages/cli/src/constructs/key-value-pair-codegen.ts(1 hunks)packages/cli/src/constructs/msteams-alert-channel-codegen.ts(1 hunks)packages/cli/src/constructs/opsgenie-alert-channel-codegen.ts(1 hunks)packages/cli/src/constructs/pagerduty-alert-channel-codegen.ts(1 hunks)packages/cli/src/constructs/phone-call-alert-channel-codegen.ts(1 hunks)packages/cli/src/constructs/private-location-codegen.ts(1 hunks)packages/cli/src/constructs/slack-alert-channel-codegen.ts(1 hunks)packages/cli/src/constructs/sms-alert-channel-codegen.ts(1 hunks)packages/cli/src/constructs/telegram-alert-channel-codegen.ts(1 hunks)packages/cli/src/constructs/webhook-alert-channel-codegen.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/cli/src/constructs/key-value-pair-codegen.ts
- packages/cli/src/constructs/telegram-alert-channel-codegen.ts
- packages/cli/src/constructs/email-alert-channel-codegen.ts
- packages/cli/src/constructs/phone-call-alert-channel-codegen.ts
- packages/cli/src/constructs/webhook-alert-channel-codegen.ts
🧰 Additional context used
🧬 Code Definitions (8)
packages/cli/src/constructs/slack-alert-channel-codegen.ts (2)
packages/cli/src/constructs/alert-channel-codegen.ts (2)
AlertChannelResource(21-29)buildAlertChannelProps(31-51)packages/cli/src/constructs/internal/codegen/context.ts (1)
Context(5-121)
packages/cli/src/constructs/api-check-codegen.ts (5)
packages/cli/src/constructs/check-codegen.ts (2)
CheckResource(17-35)buildCheckProps(37-154)packages/cli/src/constructs/internal/assertion-codegen.ts (2)
valueForNumericAssertion(4-43)valueForGeneralAssertion(45-146)packages/cli/src/constructs/internal/codegen/index.ts (2)
Codegen(1-1)Context(2-2)packages/cli/src/constructs/internal/codegen/context.ts (1)
Context(5-121)packages/cli/src/constructs/key-value-pair-codegen.ts (1)
valueForKeyValuePair(4-17)
packages/cli/src/constructs/sms-alert-channel-codegen.ts (3)
packages/cli/src/constructs/alert-channel-codegen.ts (2)
AlertChannelResource(21-29)buildAlertChannelProps(31-51)packages/cli/src/constructs/internal/codegen/index.ts (2)
Codegen(1-1)Context(2-2)packages/cli/src/constructs/internal/codegen/context.ts (1)
Context(5-121)
packages/cli/src/constructs/opsgenie-alert-channel-codegen.ts (3)
packages/cli/src/constructs/alert-channel-codegen.ts (2)
AlertChannelResource(21-29)buildAlertChannelProps(31-51)packages/cli/src/constructs/internal/codegen/index.ts (2)
Codegen(1-1)Context(2-2)packages/cli/src/constructs/internal/codegen/context.ts (1)
Context(5-121)
packages/cli/src/constructs/incidentio-alert-channel-codegen.ts (4)
packages/cli/src/constructs/webhook-alert-channel-codegen.ts (2)
WebhookAlertChannelResource(28-31)WebhookAlertChannelResourceConfig(17-26)packages/cli/src/constructs/internal/codegen/index.ts (2)
Codegen(1-1)Context(2-2)packages/cli/src/constructs/internal/codegen/context.ts (1)
Context(5-121)packages/cli/src/constructs/alert-channel-codegen.ts (1)
buildAlertChannelProps(31-51)
packages/cli/src/constructs/private-location-codegen.ts (3)
packages/cli/src/sourcegen/program.ts (1)
Program(7-53)packages/cli/src/constructs/internal/codegen/index.ts (2)
Codegen(1-1)Context(2-2)packages/cli/src/constructs/internal/codegen/context.ts (1)
Context(5-121)
packages/cli/src/constructs/check-group-codegen.ts (8)
packages/cli/src/constructs/frequency-codegen.ts (2)
FrequencyResource(4-4)valueForFrequency(6-54)packages/cli/src/constructs/alert-escalation-policy-codegen.ts (2)
AlertEscalationResource(4-4)valueForAlertEscalation(6-65)packages/cli/src/constructs/retry-strategy-codegen.ts (2)
RetryStrategyResource(4-4)valueForRetryStrategy(6-77)packages/cli/src/constructs/internal/codegen/index.ts (2)
Context(2-2)Codegen(1-1)packages/cli/src/constructs/private-location-codegen.ts (1)
valueForPrivateLocationFromId(14-23)packages/cli/src/constructs/key-value-pair-codegen.ts (1)
valueForKeyValuePair(4-17)packages/cli/src/constructs/alert-channel-codegen.ts (1)
valueForAlertChannelFromId(55-64)packages/cli/src/constructs/api-check-codegen.ts (1)
valueForAssertion(27-44)
packages/cli/src/constructs/msteams-alert-channel-codegen.ts (4)
packages/cli/src/constructs/webhook-alert-channel-codegen.ts (2)
WebhookAlertChannelResource(28-31)WebhookAlertChannelResourceConfig(17-26)packages/cli/src/constructs/internal/codegen/index.ts (2)
Codegen(1-1)Context(2-2)packages/cli/src/constructs/internal/codegen/context.ts (1)
Context(5-121)packages/cli/src/constructs/alert-channel-codegen.ts (1)
buildAlertChannelProps(31-51)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: test - windows-latest
- GitHub Check: test - ubuntu-latest
🔇 Additional comments (25)
packages/cli/src/constructs/api-check-codegen.ts (6)
8-12: Well-structured interface for snippet resourcesThe
SnippetResourceinterface provides a clear structure for script snippets with appropriate optional fields.
14-25: Good design for API check resourcesThe
ApiCheckResourceinterface properly extendsCheckResourceand provides comprehensive fields for API checks. The flexibility in specifying setup and teardown scripts gives users multiple options.
27-44: Well-implemented assertion value handlerThe
valueForAssertionfunction properly maps assertion sources to appropriate builder methods with good error handling for unsupported assertion sources.
128-128: Address TODOs with an issue or implementation planThere are TODO comments about moving scripts to separate files and using entrypoints. Consider addressing these with a tracked issue or implementation plan.
Since these TODOs appear to be related to architectural improvements, could you provide more context on when and how these improvements will be implemented? You might want to create tracked issues for these improvements to ensure they're not forgotten.
Also applies to: 144-144
48-163: Well-implemented code generation for API checksThe
ApiCheckCodegenclass thoroughly handles all aspects of API check configuration with proper conditional handling of optional properties. The use of the builder pattern ensures clean code generation.
1-7: Well-organized importsThe imports are properly structured, importing only what's needed from the relevant modules.
packages/cli/src/constructs/private-location-codegen.ts (5)
1-3: Imports follow the project's module structureThe imports correctly reference internal modules for code generation utilities, showing good organization.
12-12: Good use of constant for construct nameUsing a constant for the construct name avoids string duplication and enables easier refactoring.
14-23: Well-structured utility function for generating PrivateLocation referencesThe function properly:
- Adds the necessary import
- Returns an expression that creates a reference to an existing PrivateLocation by ID
This will be useful when linking resources that reference private locations.
25-29: Codegen class extends base class with proper type parameterThe
preparemethod correctly registers the private location ID in the context for later reference, following the pattern established in other codegen classes.
1-58:❓ Verification inconclusive
Overall implementation fits well in the import command architecture
This code generator follows the established patterns for other resource types, maintaining consistency with the rest of the codebase. It will allow users to import their existing private locations as part of the new CLI import command.
Consider implementing tests for this code generator to ensure it produces the expected output for various input configurations.
🏁 Script executed:
#!/bin/bash # Look for tests of similar codegen classes to understand test patterns echo "Searching for existing codegen test files..." fd -e ts test.*codegen packages/cli/srcLength of output: 142
Attention: Consistency and Test Coverage Verification
The implementation is consistent with established patterns for CLI import commands. However, the codebase does not currently include tests for similar code generation classes (as indicated by the absence of matching test files in the search). To ensure ongoing reliability and correctness, please implement tests for this code generator that validate its output for various input configurations.
- File Impacted:
packages/cli/src/constructs/private-location-codegen.ts- Recommendation: Add tests modeled on testing patterns from similar features or resource types to cover edge cases and generated output.
packages/cli/src/constructs/check-group-codegen.ts (3)
13-27: Solid introduction of new resource interfaces.The interfaces
BrowserCheckConfigResource,MultiStepCheckConfigResource, andCheckGroupResourcecomprehensively capture the properties needed for check group configurations, leveraging typed fields to ensure clarity in usage.
28-33:doubleCheckandconcurrencyremain unused in the code generation.Despite being defined in the
CheckGroupResource, these properties are never set inbuildCheckGroupProps. This can lead to incomplete code generation.As a follow-up, apply this diff in
buildCheckGroupPropsto include these properties:+ if (resource.doubleCheck !== undefined) { + builder.boolean('doubleCheck', resource.doubleCheck) + } + if (resource.concurrency !== undefined) { + builder.number('concurrency', resource.concurrency) + }
75-79: Emptycatchblocks swallow potential errors silently.In both the
lookupCheckGroupPrivateLocationsandlookupCheckGroupAlertChannelscalls, any lookup errors are caught and ignored. This behavior might make diagnosing errors more difficult, as issues could fail silently.Could you confirm if this is intentional? If not, consider logging or handling these errors more explicitly to surface potential problems.
Also applies to: 116-121
packages/cli/src/constructs/slack-alert-channel-codegen.ts (1)
1-47: Implementation looks good with a well-structured code generation pattern.The code follows a clean pattern for generating Slack alert channel code. The interface properly extends
AlertChannelResource, and the code generation logic correctly handles both required and optional properties.packages/cli/src/constructs/sms-alert-channel-codegen.ts (1)
15-47: Overall implementation follows the standard alert channel code generation pattern.The code is well-structured with proper separation of preparation and code generation phases. The conditional property handling for the optional name property is implemented correctly.
packages/cli/src/constructs/msteams-alert-channel-codegen.ts (2)
6-10: Clean extension of WebhookAlertChannelResource for MS Teams.The interface properly constrains the webhook type to 'WEBHOOK_MSTEAMS', ensuring type safety for MS Teams alert channels.
14-42: Well-structured code generation for MS Teams alert channel.The implementation follows the standard pattern and properly generates the MS Teams alert channel code with the required properties.
packages/cli/src/constructs/opsgenie-alert-channel-codegen.ts (3)
5-13: Comprehensive config properties for Opsgenie integration.The interface properly defines all required properties for Opsgenie integration including name, apiKey, region, and priority.
33-40: All Opsgenie properties correctly mapped in the generated code.The implementation properly maps all the configuration properties needed for the Opsgenie alert channel.
17-47: Overall implementation follows consistent alert channel pattern.The class implements the standard alert channel code generation pattern with proper preparation and code generation phases.
packages/cli/src/constructs/pagerduty-alert-channel-codegen.ts (2)
5-12: Interface definition looks sound.The optional fields and required
serviceKeyproperty are appropriately declared, and their types appear correct.
21-50: Ensure test coverage for new alert channel code generation.While the logic here is straightforward, consider adding or verifying tests to ensure that
gencodecorrectly generates aPagerdutyAlertChannelwith various valid and edge-case input configurations.packages/cli/src/constructs/incidentio-alert-channel-codegen.ts (2)
13-25: API key extraction logic appears correct.This function cleanly filters headers and extracts the Bearer token. It should work well for the intended use case.
47-47: Redundant.toString()call onconfig.url.
Affected Components
Notes for the Reviewer
New Dependency Submission