Skip to content

Make build reusable, so it can be invoked from core-v2 - #277

Merged
mroloux merged 2 commits into
masterfrom
matti/reusable-build
Sep 18, 2026
Merged

mroloux merged 2 commits into
masterfrom
matti/reusable-build

Conversation

@mroloux

@mroloux mroloux commented Sep 18, 2026

Copy link
Copy Markdown
Member

No description provided.

Copilot AI lite review requested due to automatic review settings September 18, 2026 06:54

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Critical fork checkout and secret-handling issues remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Extracts the test workflow into a reusable GitHub Actions workflow invoked by build.yml.

Changes:

  • Adds configurable refs, API URLs, Java versions, and secrets.
  • Moves the test matrix and setup steps into tests.yml.
  • Updates build.yml to call the reusable workflow.
File summaries
File Summary
.github/workflows/tests.yml Defines the reusable workflow; fork PRs cannot receive the required secret, and omitted optional secrets are exported empty, affecting test skipping.
.github/workflows/build.yml Invokes the reusable workflow; fork PR checkout may fail because the supplied SHA may not exist in the upstream repository.
Review details

Suppressed comments (1)

.github/workflows/tests.yml:54

  • This optional secret is exported unconditionally, so callers that omit it still create DEMO_COMPANY_SECRET_KEY with an empty value. SeatsioClientTest.isDemoCompanySecretKeySet() checks only whether the variable exists, causing the usage-report tests to run with an empty API key instead of being skipped when core-v2 invokes this workflow without the optional secret. Omit/unset the variable when the secret is absent, or change the test helper to require a non-empty value.
        DEMO_COMPANY_SECRET_KEY: ${{ secrets.DEMO_COMPANY_SECRET_KEY }}
  • Files reviewed: 2/2 changed files
  • Comments generated: 2
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread .github/workflows/build.yml
Comment thread .github/workflows/tests.yml Outdated
Copilot AI review requested due to automatic review settings September 18, 2026 07:08
@mroloux
mroloux marked this pull request as ready for review September 18, 2026 07:09
@mroloux
mroloux requested a review from a team as a code owner September 18, 2026 07:09
@mroloux
mroloux requested review from bverbeken and schaloner and removed request for a team September 18, 2026 07:09

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Workflow checkouts must use a ref available in the target repository for fork pull requests.

Review details

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

.github/workflows/tests.yml:41

  • The caller passes github.event.pull_request.head.sha, but this checkout is hard-coded to seatsio/seatsio-java. For a pull request opened from a fork, that SHA exists only in the fork, so checkout fails before the tests run. Make the repository an input and pass github.event.pull_request.head.repo.full_name for PRs (while retaining seatsio/seatsio-java for cross-repository callers), or use a base-repository ref instead of the fork SHA.

.github/workflows/build.yml:13

  • This passes the pull request's head SHA to a checkout of the base seatsio/seatsio-java repository. For PRs opened from forks, that SHA belongs to the fork and is not available in the base repository, so actions/checkout fails before the tests run. Use the event's github.sha/merge commit, or another ref guaranteed to exist in seatsio/seatsio-java.
      ref: ${{ github.event.pull_request.head.sha || github.sha }}
  • Files reviewed: 3/3 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@mroloux
mroloux merged commit d748d91 into master Sep 18, 2026
3 checks passed
@mroloux
mroloux deleted the matti/reusable-build branch September 18, 2026 09:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants