Imrove PR workflow #38
Conversation
… Formatter Logic' as complete
There was a problem hiding this comment.
Pull request overview
This pull request implements improvements to the PR workflow and command history analysis features, focusing on API refactoring and enhanced filtering capabilities.
Changes:
- Refactored GitHub
ListPRsAPI from two separate parameters to a structuredListPRsOptionstype with pagination support - Enhanced history query engine with new filtering options:
MinDuration,SessionID, andProjectPaths - Added formatted timeline output with visual indicators (✅/❌) for command success/failure
- Moved
CommandandQueryOptionstypes fromdatabase.goto dedicatedtypes.gofile - Comprehensive conductor documentation for project workflow and tech stack
Reviewed changes
Copilot reviewed 48 out of 48 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/github/types.go | Added ListPRsOptions struct with State, Author, Limit, and Page fields |
| pkg/github/client.go | Updated ListPRs interface method signature to use options struct |
| pkg/github/api_client.go | Implemented pagination support using the new options struct |
| pkg/github/cli_client.go | Updated CLI client with pagination (limit only, page parameter noted as unsupported) |
| pkg/workflow/workflow_test.go | Updated mock to match new ListPRs signature |
| pkg/history/types.go | Moved and extended Command and QueryOptions type definitions |
| pkg/history/database.go | Removed type definitions (moved to types.go), enhanced query builders with new filters |
| pkg/history/template.go | New timeline formatter with visual indicators and summary statistics |
| pkg/history/template_test.go | Tests for new timeline formatting functionality |
| pkg/history/integration_test.go | Integration tests for new query filters |
| pkg/history/builder_new_test.go | Unit tests for query builder with new filters (has compilation issue) |
| pkg/history/query_options_test.go | Compilation test for new QueryOptions fields |
| pkg/git/worktree.go | Added GetWorktreePath helper method |
| cmd/timeline.go | Refactored to use new timeline formatter and support project path filtering |
| cmd/timeline_test.go | Removed old timeline generation tests (moved to pkg/history) |
| cmd/timeline_flags_test.go | Tests for new timeline command flags |
| cmd/history.go | Added flags for new query filters |
| cmd/history_flags_test.go | Tests for new history command flags |
| cmd/pr_*.go | Refactored PR commands to use options structs instead of global variables |
| cmd/pr_*_test.go | New unit tests for refactored PR commands |
| conductor/* | New project documentation including workflow, tech stack, and track specifications |
| GEMINI.md | Project overview documentation for AI context |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| query, args := dm.buildZshHistdbQuery(options) | ||
|
|
||
| expected := "c.duration >=" | ||
| if !containsString(query, expected) { |
There was a problem hiding this comment.
The test file uses the helper function containsString which is not defined in this file. This function is defined in database_test.go but is not accessible here. You should either move the helper function to a shared test utility file, or use strings.Contains from the standard library instead.
| func TestQueryOptions_NewFields(t *testing.T) { | ||
| // This test will fail to compile if fields are missing | ||
| opts := history.QueryOptions{ | ||
| MinDuration: 5 * time.Second, // Should fail compilation |
There was a problem hiding this comment.
The comment on line 13 states "Should fail compilation" but this appears to be incorrect. The test is checking that the field exists and compiles successfully, not that it fails to compile. The comment should be corrected to something like "Will fail to compile if MinDuration field is missing".
No description provided.