Skip to content

refactor: code simplification and consistency improvements#40

Merged
rianjs merged 1 commit intomainfrom
refactor/39-code-simplification
Feb 2, 2026
Merged

refactor: code simplification and consistency improvements#40
rianjs merged 1 commit intomainfrom
refactor/39-code-simplification

Conversation

@rianjs
Copy link
Contributor

@rianjs rianjs commented Feb 2, 2026

Summary

  • Extract duplicate formatBool function to shared package (internal/cmd/shared/format.go)
  • Add --force flag to schemas delete command with safety warning
  • Add --force flag to workflows delete command with safety warning
  • Fix GetRootTypes to properly check for GraphQL internal types (__ prefix instead of _)
  • Simplify ErrorMessages using strings.Join instead of loop concatenation
  • Refactor inline interface types to named tableRenderer interface in graphql.go

Test plan

  • All existing tests pass
  • Added unit test for FormatBool in shared package
  • Lint passes with no issues
  • Build succeeds

Closes #39

🤖 Generated with Claude Code

- Extract duplicate formatBool to shared package
- Add --force flag to schemas delete command for safety
- Add --force flag to workflows delete command for safety
- Fix GetRootTypes to check for double underscore (__) prefix
- Simplify ErrorMessages with strings.Join
- Refactor inline interfaces to named tableRenderer type

Closes #39
@rianjs
Copy link
Contributor Author

rianjs commented Feb 2, 2026

Test Coverage Assessment

Summary

This PR contains a mix of refactoring and new functionality. The test coverage is acceptable overall but has a notable gap for the new --force flag behavior.


Changes and Coverage Analysis

1. shared.FormatBool - Adequately Tested

The new internal/cmd/shared/format.go function has corresponding tests in format_test.go covering both true and false cases. This is appropriate for such a simple utility function.

2. ErrorMessages() refactoring - Already Covered

The change from loop concatenation to strings.Join is already well-covered by TestGraphQLResponse_ErrorMessages in api/graphql_test.go, which tests:

  • No errors (empty string)
  • Single error
  • Multiple errors with semicolon separator

3. GetRootTypes() fix - Already Covered

The change from t.Name[0:1] != "_" to strings.HasPrefix(t.Name, "__") is covered by TestIntrospectionSchema_GetRootTypes. The test includes a __Schema type that verifies internal types are filtered out.

4. tableRenderer interface extraction - No Test Impact

This is purely a code organization change (extracting an inline interface to a named type). No functional change, no test needed.

5. --force flag for delete commands - NOT Tested

The new safety feature adds --force flags to both schemas delete and workflows delete commands with the following behavior:

  • Without --force: Shows warning message, returns without deleting
  • With --force: Proceeds with deletion

This is the only meaningful gap. The internal/cmd/ packages have no command-level tests currently, so this follows the existing pattern. However, this is new user-facing behavior that could benefit from tests verifying:

  • Without --force, the command returns nil and doesn't call the API
  • With --force, the command proceeds to call DeleteSchema/DeleteWorkflow

Recommendation

Merge as-is. The lack of command-level tests for the --force flag is consistent with the rest of the codebase (no internal/cmd/*_test.go files exist). The underlying API layer (api/) is well-tested. Adding command-level tests would be valuable as a separate enhancement but isn't blocking for this refactoring PR.

If you prefer to add tests before merging, consider a follow-up task to add command-level testing infrastructure with the --force behavior as the first test case.

@rianjs rianjs merged commit 7c17bc3 into main Feb 2, 2026
3 checks passed
@rianjs rianjs deleted the refactor/39-code-simplification branch February 2, 2026 11:03
@rianjs rianjs mentioned this pull request Feb 2, 2026
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

refactor: Code simplification and consistency improvements

1 participant

Comments