-
Notifications
You must be signed in to change notification settings - Fork 1
Convert to library: extract core functionality to pkg/codingcontext #105
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Co-authored-by: alexec <1142830+alexec@users.noreply.github.com>
Co-authored-by: alexec <1142830+alexec@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR successfully refactors a monolithic CLI tool into a reusable Go library by extracting core functionality into pkg/codingcontext while maintaining the CLI as a thin wrapper (~400 to ~70 lines). The refactoring creates a clean public API using the functional options pattern and maintains backward compatibility with all existing CLI behavior.
Key Changes:
- Introduced public API with
Contexttype and functional options pattern (New(),WithWorkDir(), etc.) - Exported core types (
Params,Selectors,FrontMatter) and utility functions (ParseMarkdownFile,EstimateTokens,AllRulePaths, etc.) - Moved unit tests to
pkg/codingcontext/*_test.gowhile preserving integration tests in the root
Reviewed Changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pkg/codingcontext/types.go | Defines public types including Context, Params, Selectors, and FrontMatter |
| pkg/codingcontext/context.go | Implements the main Context API with functional options and Run() method |
| pkg/codingcontext/remote.go | Exports DownloadRemoteDirectory for remote rule/task fetching |
| pkg/codingcontext/paths.go | Exports path discovery functions (AllRulePaths, AllTaskSearchPaths, etc.) |
| pkg/codingcontext/selectors.go | Exports Selectors type with methods for frontmatter filtering |
| pkg/codingcontext/params.go | Exports Params type for template parameter substitution |
| pkg/codingcontext/markdown.go | Exports ParseMarkdownFile for frontmatter parsing |
| pkg/codingcontext/token_counter.go | Exports EstimateTokens utility function |
| pkg/codingcontext/*_test.go | Updated test files with renamed types and functions |
| pkg/codingcontext/README.md | Comprehensive library documentation with usage examples |
| main.go | Refactored CLI to delegate to pkg/codingcontext library |
| remote.go | Removed (functionality moved to pkg/codingcontext/remote.go) |
| frontmatter.go | Removed (type definition moved to pkg/codingcontext/types.go) |
| integration_test.go | Minor comment update to reflect new naming |
|
@swedishborgie for your review regarding the interface. I think it looks good to me. You can comment on the impl if you like - but I'd not bother. |
Instead of changing the current working directory which affects the entire process and is not thread-safe, the library now resolves all relative paths based on the workDir parameter. Updated AllTaskSearchPaths and AllRulePaths to accept a baseDir parameter and properly join all relative paths with that directory. This makes the library safe to use in multi-threaded applications and doesn't have unexpected side effects on the process state. Co-authored-by: alexec <1142830+alexec@users.noreply.github.com>
|
@copilot please fetch and merge master and fix the conflicts. Look at the |
- Renamed RuleContent to Markdown - Added FrontMatter field to Markdown type - Changed Task field in Result from string to Markdown - Removed TaskFrontmatter field from Result (now in Task.FrontMatter) - Updated all code, tests, and documentation to use new structure This provides a more consistent API where both rules and tasks are represented the same way, including their frontmatter metadata. Co-authored-by: alexec <1142830+alexec@users.noreply.github.com>
|
@copilot merge main and fix the conflicts |
…into copilot/turn-into-library-pkg
|
@copilot fix the failing check |
The deadcode linter was reporting ParseSlashCommand as unreachable because it's a public API function in a library package. Adding the -test flag includes test executables as entry points, which properly recognizes that library functions used in tests are not dead code. This is the appropriate configuration for a project that provides library packages (pkg/codingcontext and pkg/slashcommand) with public APIs intended for external use. Co-authored-by: alexec <1142830+alexec@users.noreply.github.com>
Head branch was pushed to by a user without write access
Fixed in commit ef9567e. Added |
|
@copilot fix the failing test. |
The library was running task bootstrap scripts internally, which caused the stderr output to appear before stdout content was written by main.go. This broke the expected ordering in integration tests. Changes: - Removed task bootstrap execution from library Run() method - Added BootstrapPath() method to Markdown type - main.go now runs task bootstrap between outputting rules and task - This preserves the correct output order: rule bootstrap, rule content, task bootstrap, task content All tests now pass including TestTaskBootstrapWithRuleBootstrap. Co-authored-by: alexec <1142830+alexec@users.noreply.github.com>
Head branch was pushed to by a user without write access
Fixed in commit fb56cbf. The issue was that the library was running task bootstrap scripts internally, causing stderr output to appear before stdout content. Moved task bootstrap execution to main.go (after outputting rules, before outputting task) to preserve the correct output ordering. Added |
Converting coding-context-cli into a reusable library
Completed:
pkg/codingcontextdirectory structureContextstruct and its methodsParams,Selectors,FrontMattertypesNew()with functional optionsLatest Changes:
Fixed test failure by moving task bootstrap execution to CLI:
BootstrapPath()method toMarkdowntype for callers to get bootstrap pathSummary:
Successfully converted coding-context-cli into a reusable Go library:
Library Package (
pkg/codingcontext)Resulttype withRules []MarkdownandTask MarkdownContext,Result,Markdown,Params,Selectors,FrontMatter(each in its own file)New(),Run()returns(*Result, error),ParseMarkdownFile(),AllRulePaths(),AllTaskSearchPaths()Markdown.BootstrapPath()for getting bootstrap script path${param}substitutionMarkdowntype for representing both rules and tasks with frontmatterAdditional Package (
pkg/slashcommand)ParseSlashCommand()CLI (
main.go)Testing & CI
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.