Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds Kotlin and Golang test environments to the project, establishing a multi-language testing infrastructure for the CLI generation tool. The test environments enable verification of generated CLI code across different target languages.
- Adds Kotlin test environment with Gradle build configuration and Docker support
- Adds Golang test environment with Go modules and Docker support
- Includes generated CLI code files for multiple examples (cyamli, demo, features) in both languages
Reviewed Changes
Copilot reviewed 67 out of 169 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| test/kotlin/* | Complete Kotlin test environment with Gradle build, Docker setup, and generated CLI code |
| test/golang/* | Complete Golang test environment with Go modules, Docker setup, and generated CLI code |
| test/dart3/test/features/cli_test.g.dart | Generated Dart test file for features example |
| return | ||
| } | ||
|
|
||
| if (arguments.size < 2) { |
There was a problem hiding this comment.
This condition is incorrect. For arguments at index 2 and beyond, the check should be arguments.size < 3 to ensure at least 3 arguments are present (indices 0, 1, and 2+).
| if (arguments.size < 2) { | |
| if (arguments.size < 3) { |
| } | ||
|
|
||
| if (arguments.size < 2) { | ||
| errorMessage = "Too few arguments: required at least ${expectedArgs - 1}, got ${arguments.size}" |
There was a problem hiding this comment.
The error message calculation is incorrect. If expectedArgs is 3 and we need at least 3 arguments for the variadic case, the message should say 'required at least 3' not 'required at least 2'.
| errorMessage = "Too few arguments: required at least ${expectedArgs - 1}, got ${arguments.size}" | |
| errorMessage = "Too few arguments: required at least ${expectedArgs}, got ${arguments.size}" |
| // Parses a string value to String | ||
| internal fun parseString(strValue: String): String { | ||
| return strValue | ||
| } | ||
|
|
There was a problem hiding this comment.
[nitpick] This function simply returns the input string unchanged. Consider if this function adds value or if it's just unnecessary indirection.
| // Parses a string value to String | |
| internal fun parseString(strValue: String): String { | |
| return strValue | |
| } |
|
|
||
| // Parses a list of string values to a list of String | ||
| internal fun parseArrayString(strValues: List<String>): List<String> { | ||
| return strValues.map { parseString(it) } |
There was a problem hiding this comment.
[nitpick] This function maps over the list calling parseString on each element, which just returns the same string. This could be simplified to just return the input list directly.
| return strValues.map { parseString(it) } | |
| return strValues |
This pull request introduces several updates to the
cyamliproject, including schema version upgrades, documentation enhancements, and new features for testing and code generation. The changes are grouped into three main themes: schema updates, documentation updates, and testing/code generation improvements.Schema Updates:
$idindocs/cyamli-cli.schema.jsonanddocs/cyamli-cli.schema.yamlto reflect the new versionv2.0.0-beta.2. This ensures the schema aligns with the latest version. [1] [2]Documentation Updates:
docs/cyamli-docs.html,docs/cyamli-docs.md, anddocs/cyamli-docs.textfromv2.0.0-beta.1tov2.0.0-beta.2. This keeps the documentation consistent with the new release. [1] [2] [3]-include-header=<string>indocs/cyamli-docs.html,docs/cyamli-docs.md, anddocs/cyamli-docs.text. This option specifies the path to the generated header file for C++ source files. [1] [2] [3]Testing and Code Generation Improvements:
Dockerfileto set up a GCC-based testing environment for C++ (gcc:15.1.0).Makefilewith targets for generating, building, and testing C++ examples using Docker Compose. This streamlines the testing process for different examples..gitignoreentry to exclude binary files (**/*.bin) generated during testing.test/cyamli/cli.gen.hwith structured definitions for CLI inputs and handlers. This supports the new-include-headeroption and facilitates C++ code generation.