Conversation
…ntainability - Cleaned up imports and exports in checks.ts, checksAPI.ts, checksConstants.ts, checksFilters.ts, and checksInterfaces.ts. - Consolidated and optimized functions for filtering and processing checks based on app ID and name. - Introduced new utility functions to handle duplicate entries and check statuses. - Added new statuses module with interfaces and constants for managing commit statuses. - Removed commented-out code to enhance clarity and focus on active functionality.
- Implement getMostRecentStatusPerContextAndCreator function to filter statuses. - Update getAllStatusCommits to return typed IStatus[]. - Create tests for status filtering logic. - Add documentation for filtering by commit status.
…prove code organization and readability
There was a problem hiding this comment.
Pull Request Overview
This PR introduces support for GitHub commit statuses alongside checks when evaluating commit state. The action can now optionally fetch and process commit statuses through a new include_status_commits input parameter.
- Adds new input parameter
include_status_commitsto enable/disable commit status evaluation - Implements commit status fetching, filtering, and mapping to the existing checks model
- Enhances summary output to distinguish between checks and commit statuses with separate tables
Reviewed Changes
Copilot reviewed 15 out of 41 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils/inputsExtractor.ts | Adds new boolean input includeStatusCommits |
| src/statuses/statusesInterfaces.ts | Defines interface for GitHub commit status objects |
| src/statuses/statusesFilters.ts | Implements filtering logic to get most recent status per context/creator |
| src/statuses/statusesConstants.ts | Defines commit status states and emoji mapping functions |
| src/statuses/statusesAPI.ts | Provides API client for fetching commit statuses |
| src/statuses/statuses.ts | Maps commit statuses to the existing checks model |
| src/checks/checksInterfaces.ts | Refactors interfaces and adds optional commit_status field |
| src/checks/checksFilters.ts | Code formatting improvements without functional changes |
| src/checks/checksConstants.ts | Removes duplicate commit status constants and formatting cleanup |
| src/checks/checksAPI.ts | Adds proper typing and removes commented code |
| src/checks/checks.ts | Integrates commit status fetching and enhances summary display |
| src/checks/checkEmoji.ts | Formatting improvements for object properties |
| docs/adrs/commit_status.md | Placeholder documentation for commit status filtering |
| action.yml | Adds new input parameter definition |
| tests/statuses/statusesFilters.test.ts | Comprehensive test coverage for status filtering logic |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
src/statuses/statusesConstants.ts
Outdated
Comment on lines
+8
to
+12
| interface ICommitStatuseEmoji { | ||
| [key: string]: string; | ||
| } | ||
|
|
||
| const commitStatusStateEmojis: ICommitStatuseEmoji = { |
There was a problem hiding this comment.
Corrected spelling of 'ICommitStatuseEmoji' to 'ICommitStatusEmoji'.
Suggested change
| interface ICommitStatuseEmoji { | |
| [key: string]: string; | |
| } | |
| const commitStatusStateEmojis: ICommitStatuseEmoji = { | |
| interface ICommitStatusEmoji { | |
| [key: string]: string; | |
| } | |
| const commitStatusStateEmojis: ICommitStatusEmoji = { |
src/statuses/statusesConstants.ts
Outdated
Comment on lines
+8
to
+12
| interface ICommitStatuseEmoji { | ||
| [key: string]: string; | ||
| } | ||
|
|
||
| const commitStatusStateEmojis: ICommitStatuseEmoji = { |
There was a problem hiding this comment.
Interface name should be corrected to 'ICommitStatusEmoji' to match the fix above.
Suggested change
| interface ICommitStatuseEmoji { | |
| [key: string]: string; | |
| } | |
| const commitStatusStateEmojis: ICommitStatuseEmoji = { | |
| interface ICommitStatusEmoji { | |
| [key: string]: string; | |
| } | |
| const commitStatusStateEmojis: ICommitStatusEmoji = { |
docs/adrs/commit_status.md
Outdated
| @@ -0,0 +1,3 @@ | |||
| ## Filtering by Commit Status | |||
|
|
|||
| There are some nuances to be aware of when it comes to filtering by commit statuses using this action. No newline at end of file | |||
There was a problem hiding this comment.
The documentation file is incomplete. It only contains a header with no actual content explaining the nuances of filtering by commit statuses.
Suggested change
| There are some nuances to be aware of when it comes to filtering by commit statuses using this action. | |
| Filtering by commit status allows you to select commits based on the state of their associated status checks (such as CI/CD pipelines, code quality checks, or other automated processes). This can be useful for automating workflows that depend on the success or failure of these checks. | |
| ### What is a Commit Status? | |
| A commit status is a label (such as `success`, `failure`, or `pending`) attached to a commit by external systems (like continuous integration services). Each status is associated with a context (e.g., `ci/travis`, `codecov/project`) and a state. | |
| ### Nuances and Caveats | |
| - **Multiple Status Contexts:** A single commit can have multiple status contexts. When filtering, you may need to specify which context(s) you care about, or whether you want all statuses to be successful. | |
| - **Status vs. Check Runs:** Some platforms distinguish between "statuses" and "check runs." Make sure you are filtering the correct type for your use case. | |
| - **Pending Statuses:** If a status is `pending`, it may not be safe to assume the commit is ready for further action. Consider whether to include or exclude pending statuses in your filter. | |
| - **Timing Issues:** Statuses may be updated asynchronously. A commit that initially has a `pending` status may later be updated to `success` or `failure`. Ensure your workflow accounts for this delay. | |
| - **Required Statuses:** Some repositories enforce required status checks before allowing merges. Filtering by commit status can help enforce these policies programmatically. | |
| ### Example Usage | |
| To filter commits that have passed all required status checks: | |
| ```yaml | |
| - uses: actions/checkout@v3 | |
| - name: Filter commits by status | |
| uses: your-org/filter-commits-by-status@v1 | |
| with: | |
| status: success | |
| context: ci/travis |
In this example, only commits with a success status in the ci/travis context will be selected.
Best Practices
- Always specify the status context if your repository uses multiple CI/CD systems.
- Consider re-checking commit statuses if your workflow is triggered by events that may occur before all statuses are available.
- Document which statuses are required for your workflow to avoid confusion.
- Updated tests for checks filtering functions to improve readability and consistency. - Added comprehensive tests for `getMostRecentStatusPerContextAndCreator` to ensure correct behavior with multiple contexts and creators. - Introduced new tests for `mapStatusesToChecksModel` to validate mapping of various status states and ensure preservation of original status fields. - Implemented tests for `getAllStatusCommits` to verify API interaction and error handling. - Added tests for constants related to commit status to ensure correct emoji representation and state validation.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 27 out of 54 changed files in this pull request and generated no new comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #61
This pull request introduces support for including commit statuses alongside checks when evaluating the state of a commit. It adds a new input to the action, updates the core logic to fetch and process commit statuses, and improves the summary output to clearly distinguish between checks and commit statuses. The changes also include refactoring for code clarity and test coverage for the new filtering logic.