-
Notifications
You must be signed in to change notification settings - Fork 2
Open
Description
Problem
The current bash implementation requires:
- Manual API URL construction
- curl + jq for API calls and JSON parsing
- Complex string manipulation for extracting PR numbers (e.g.,
sed -n 's|^queue/[^/]*/pr-\([0-9]\+\).*|\1|p') - Multiple fallback strategies that are harder to maintain in bash
- Limited testability (requires GitHub Actions environment)
- Harder for JS/TS developers to contribute and maintain
Proposed Solution
Convert the action from a composite (bash) action to a Node.js action using:
@actions/corefor GitHub Actions integration@actions/githubwith Octokit for clean API calls- TypeScript for type safety and better maintainability
- Native JavaScript for better error handling
Benefits
Organizational
- Team alignment: Organization is stronger with JS/TS than bash
- Easier contributions: More developers can review and contribute
- Better code reviews: TypeScript makes intent clearer
- Consistency: Aligns with other actions/tooling in the organization
Technical
- Cleaner API calls: Use Octokit SDK instead of curl + jq
// Instead of: curl + jq parsing const { data: pulls } = await octokit.rest.repos.listPullRequestsAssociatedWithCommit({ owner, repo, commit_sha: sha }); pr = pulls.length > 0 ? pulls[0].number.toString() : null;
- Better error handling: Native try/catch and error objects
- More maintainable: Easier to read and modify logic
- No external dependencies: No need for jq in the runner
- Type safety: TypeScript catches errors at compile time
- Testability: Unit tests with mocked GitHub API
- Simpler string manipulation: Native regex instead of sed/grep chains
Readability Example
Bash (current):
pr=$(echo "${MERGE_GROUP_HEAD_REF}" | sed -n 's|^queue/[^/]*/pr-\([0-9]\+\).*|\1|p')TypeScript (proposed):
const match = headRef.match(/^queue\/[^\/]+\/pr-(\d+)/);
const pr = match ? match[1] : null;Migration Plan
Phase 1: Setup
- Create
package.jsonwith dependencies:@actions/core@actions/githubtypescript@types/node@vercel/ncc(for bundling)
- Add TypeScript config (
tsconfig.json) - Add build scripts and GitHub Actions workflow for building
Phase 2: Implementation
- Create
src/main.tswith TypeScript implementation - Port all event handlers (pull_request, merge_group, push, release, workflow_dispatch)
- Implement helper functions:
getPrFromApi()using OctokitgetPrFromGit()usingsimple-gitorexeclogDebug()using@actions/core
- Update
action.ymlto usenode16ornode20instead ofcomposite
Phase 3: Testing
- Add unit tests with Jest/Vitest
- Mock GitHub API responses
- Test all event types
- Update existing workflow tests to verify compatibility
Phase 4: Rollout
- Create feature branch
- Test in staging/test workflows
- Merge and release as minor version (backward compatible)
- Monitor for issues
File Structure
action-get-pr/
├── src/
│ └── main.ts # Main action logic
├── __tests__/
│ └── main.test.ts # Unit tests
├── dist/
│ └── index.js # Bundled output (git-ignored)
├── action.yml # Updated to use node
├── package.json
├── tsconfig.json
└── .github/workflows/
└── build.yml # Build and test workflow
Considerations
- Action size: Will be slightly larger, but Node.js is already in runners
- Build step: Need to bundle with
nccand commitdist/index.js - Backward compatibility: Should maintain same inputs/outputs
- Migration effort: ~108 lines of bash → ~200-300 lines of TypeScript (with tests)
Related
This would simplify the implementation added in #29 for workflow_dispatch support and improve maintainability for the team.
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
No labels
Type
Projects
Status
Next