Skip to content

Conversation

@lucasconti-dev
Copy link

Noticed list_artifact did not expose pagination params in the schema despite supporting them, this PR matches existing patterns like in list_jobs

- Add TestListArtifactsPagination with tests for first page, second page, last page, and beyond available data
- Add TestListArtifactsPaginationDefaults to verify default pagination behavior
- Tests verify that pagination parameters are properly passed to the underlying API client
- Follows same testing pattern as other list functions (jobs, builds, etc.)
@Copilot Copilot AI review requested due to automatic review settings September 18, 2025 17:58
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 exposes pagination parameters in the list_artifacts schema to match existing patterns used in other endpoints like list_jobs. The change allows clients to specify page and perPage parameters when listing artifacts.

  • Adds pagination parameters (page and perPage) to the list_artifacts tool schema
  • Implements comprehensive test coverage for pagination functionality including edge cases
  • Maintains consistency with existing pagination patterns across the codebase

Reviewed Changes

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

File Description
pkg/buildkite/artifacts.go Adds page and perPage parameters to the list_artifacts tool schema with appropriate validation
pkg/buildkite/artifacts_test.go Adds comprehensive test coverage for pagination functionality including edge cases and default behavior

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@wolfeidau
Copy link
Member

@lucasconti-dev I have unblocked the pipeline and it highlighted a couple of linting issues, could you resolve those please.

This looks like a good improvement so I would love to get it merged and test it out.

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.

2 participants