Skip to content

feat: Allow filtering by platform on workflow call#209

Open
juanjosevazquezgil wants to merge 36 commits intomainfrom
feat/831-allow-platform-filter
Open

feat: Allow filtering by platform on workflow call#209
juanjosevazquezgil wants to merge 36 commits intomainfrom
feat/831-allow-platform-filter

Conversation

@juanjosevazquezgil
Copy link
Contributor

@juanjosevazquezgil juanjosevazquezgil commented Feb 19, 2026

Closes https://github.com/prefapp/features/issues/831

Examples (platforms)

Build for platform linux/amd64:

Build for all platforms using *:

Default (also builds for all platforms when no platforms attribute is set):

No flavors to build after filtering:

Examples (runs_on)

Default (no value for runs_on):

runs_on with value:

@juanjosevazquezgil juanjosevazquezgil requested a review from a team as a code owner February 19, 2026 13:29
Copy link

Copilot AI left a 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 adds the ability to filter which platforms (e.g., linux/amd64, linux/arm64) to build for when calling the build_images workflow. This allows callers to selectively build images for specific platforms rather than always building for all platforms defined in the configuration.

Changes:

  • Added a new platforms input parameter to the GitHub workflow with wildcard (*) or comma-separated platform list support
  • Implemented platform filtering logic that intersects requested platforms with each flavor's configured platforms
  • Added platforms property to the BuildImages class to store and access the platform filter

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
.github/workflows/build_images.yaml Added new platforms input parameter with default "*" and passes it to the workflow execution
firestarter/workflows/build_images/build_images.py Implemented platform filtering logic including platforms property, check_if_build_all_platforms method, and intersection-based filtering in compile_images_for_all_flavors
firestarter/tests/test_build_images_functionality.py Added platforms field to test variables to support the new BuildImages constructor parameter

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +493 to +495
def check_if_build_all_platforms(self):
return self.platforms.replace(' ', '') == '*'

Copy link

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test for the wildcard platform '' is missing. While test_validate_platforms tests that '' is accepted as valid input, and test_compile_images_filtering_by_platform tests the filtering behavior with '', neither test explicitly verifies that validate_platforms('') returns '' correctly or that check_if_build_all_platforms() returns True when platforms is ''.

Consider adding a specific test case:

def test_check_if_build_all_platforms():
    builder._platforms = '*'
    assert builder.check_if_build_all_platforms() == True
    
    builder._platforms = 'linux/amd64'
    assert builder.check_if_build_all_platforms() == False

This would ensure the wildcard logic is explicitly tested, though the current indirect coverage through the integration test is sufficient for basic functionality.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants