-
Notifications
You must be signed in to change notification settings - Fork 32
Refactor: Extract duplicate staged mode preview rendering into shared utility #3277
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
- Created shared staged_preview.cjs utility module with generateStagedPreview function - Updated 5 handler files to use shared utility instead of duplicate code - Reduced code by ~4 lines net (110 insertions, 114 deletions) - Added comprehensive tests for shared utility (9 test cases) - Fixed create_issue.test.cjs to match new behavior - All 708 tests passing - Formatting and linting verified Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
All tests passing (708/708) - JavaScript tests: 38 files, 708 tests - Go unit tests: All passing - Build: Successful - Formatting: Verified - Linting: Passed Benefits achieved: - Eliminated duplicate code in 5 handlers - Created reusable utility with 9 test cases - Improved maintainability and consistency - Foundation for future enhancements Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot render summary to core.info as well |
- Log full summary content to core.info for better visibility - Update tests to verify summary content logging - Addresses feedback from @pelikhan Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
Agentic Changeset Generator triggered by this pull request. |
|
... |
|
@copilot You must update the compiler and bundled JavaScript files to inline the new file. See error: https://github.com/githubnext/gh-aw/actions/runs/19117338988/job/54629867586#step:4:1 |
- Embed staged_preview.cjs file in js.go - Add to GetJavaScriptSources() map for bundling - Fixes bundling error for scripts that require staged_preview Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Fixed in f02cbbe. Added |
|
Agentic Changeset Generator triggered by this pull request. |
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 refactors duplicate staged mode preview generation logic across multiple workflow scripts by introducing a reusable generateStagedPreview utility function. The refactoring reduces code duplication and standardizes how staged mode previews are generated and displayed.
Key changes:
- Introduces a new
staged_preview.cjsmodule with agenerateStagedPreviewfunction - Refactors five workflow scripts to use the new utility instead of inline logic
- Updates test expectations to match the new standardized preview messages
Reviewed Changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/js/staged_preview.cjs | New utility module providing reusable generateStagedPreview function |
| pkg/workflow/js/staged_preview.test.cjs | Comprehensive test suite for the new utility |
| pkg/workflow/js/update_issue.cjs | Refactored to use generateStagedPreview |
| pkg/workflow/js/push_to_pull_request_branch.cjs | Refactored to use generateStagedPreview |
| pkg/workflow/js/create_pr_review_comment.cjs | Refactored to use generateStagedPreview |
| pkg/workflow/js/create_issue.cjs | Refactored to use generateStagedPreview |
| pkg/workflow/js/add_labels.cjs | Refactored to use generateStagedPreview |
| pkg/workflow/js/create_issue.test.cjs | Updated test assertion for new message format |
| pkg/workflow/js.go | Registered new script in Go embed mappings |
| .github/workflows/*.lock.yml | Auto-generated workflow files updated with new function |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@copilot merge main, format and recompile, fix tests |
…ot/remove-duplicate-staged-mode-code
- Merged latest changes from main branch - Ran code formatting (make fmt) - Recompiled all workflow lock files (make recompile) - All 710 JavaScript tests passing - All Go unit tests passing - Build successful Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Summary
Successfully refactored duplicate staged mode preview rendering code across 5 JavaScript handlers into a shared utility module.
Changes Made
New Files
pkg/workflow/js/staged_preview.cjs- Shared utility module (31 lines)pkg/workflow/js/staged_preview.test.cjs- Test suite (9 comprehensive tests)Updated Files
pkg/workflow/js/create_issue.cjs- Now uses shared utility (-10 lines)pkg/workflow/js/update_issue.cjs- Now uses shared utility (-4 lines)pkg/workflow/js/add_labels.cjs- Now uses shared utility (+1 line but cleaner)pkg/workflow/js/create_pr_review_comment.cjs- Now uses shared utility (-1 line)pkg/workflow/js/push_to_pull_request_branch.cjs- Now uses shared utility (-1 line)pkg/workflow/js/create_issue.test.cjs- Updated test expectationspkg/workflow/js.go- Added staged_preview.cjs to embedded sourcesLatest Updates
core.infologging for full summary content (addresses @pelikhan feedback)Verification
Benefits
Statistics
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.