Add create run logs#68
Conversation
|
Claude finished @ramilamparo's task in 1m 17s —— View job Review: PR #68 — Add create run logs
Result: Approved — clean, well-structured addition that follows all established codebase patterns. No security, performance, or code quality issues found. One minor inline suggestion about adding a |
AndrianBdn
left a comment
There was a problem hiding this comment.
I think it would be great to explain what is run log - it is term we coined ourself, there is no alternatives as far as I know. AI system / user could confuse this with something else, so both in SKILL.md / README.md
Clarify what run-level logs are used for in --help, README, and SKILL.md command tree.
|
Claude finished @ramilamparo's task in 2m 3s —— View job Review: PR #68 — Add create run logs
Result: Approved — clean, well-structured addition that follows all established codebase patterns. No security, performance, or code quality issues found. The new commit addresses @AndrianBdn's feedback by adding a clear explanation of what run-level logs are in both README.md and SKILL.md. One minor observation (not blocking) noted in the review about |
- Inline runs API methods into the returned object literal and drop unused long-form aliases (createRun, listRuns, etc.) to match the pattern used by folders/tcases. - Require non-empty comment in CreateRunLogRequestSchema and add tests covering both --comment and --body paths.
|
Claude finished @ramilamparo's task in 5m 3s —— View job Review: PR #68 — Add create run logs
Result: Approved — clean, well-structured addition that follows all established codebase patterns. The latest commit addresses prior feedback ( |
hi-rai
left a comment
There was a problem hiding this comment.
Looks okay, apart from few claude comments that I think would be good to address
|
Claude finished @satvik007's task in 5m 11s —— View job Review: PR #68 — Add create run logs
Result: Approved — clean, well-structured addition that follows all established codebase patterns. No security, performance, or blocking code quality issues found. Three previously posted inline suggestions remain unaddressed (as noted by @hi-rai): |
There was a problem hiding this comment.
Clean, well-structured addition that follows all established codebase patterns. No security, performance, or blocking code quality issues found.
Three previously posted inline suggestions remain unaddressed (as noted by @hi-rai):
-
import typeforCreateRunLogRequest(src/commands/api/manifests/runs.ts:8) — Used only as a type assertion; should betype CreateRunLogRequestor replaced withParameters<typeof api.runs.createLog>[2]to match peer endpoints. -
Use
expectValidationErrorhelper (src/tests/api/runs/logs-create.spec.ts:93-108) — The "rejects empty --comment" test manually reimplements the spy/assert/restore pattern that's already imported and used earlier in the same file. -
Add
missing-subcommand-helpentry (src/tests/missing-subcommand-help.spec.ts) —{ args: ['api', 'runs', 'logs'], label: 'api runs logs' }should be added to keep help-text test coverage consistent with other intermediate command groups.
Resovles: https://github.com/Hypersequent/tms-issues/issues/2471