Conversation
|
Hello! Thank you for your contribution. Please review our contribution guidelines to understand the project's testing and code conventions. |
There was a problem hiding this comment.
Pull request overview
This PR removes the github/actions client package, which previously contained the HTTP client for interacting with the GitHub Actions service, and replaces it with the external github.com/actions/scaleset package. The github/actions package is reduced to only contain the GitHubConfig URL parsing logic (in the new actions.go file), which is still used across the codebase for metrics labels and client configuration.
Changes:
- Deletes the entire Actions HTTP client (
Client,MultiClient,ActionsServiceinterface, types, errors, mocks, fakes, test helpers, test data, and tests) fromgithub/actions/. - Replaces error handling in controllers to use
scaleset.JobStillRunningErrorandscaleset.RunnerExistsErrorsentinel errors instead of inspectingActionsErrorstatus codes and exception names. - Retains only
GitHubConfigURL parsing utilities in the newgithub/actions/actions.go.
Reviewed changes
Copilot reviewed 34 out of 35 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
github/actions/actions.go |
New file retaining GitHubConfig and ParseGitHubConfigFromURL |
github/actions/client.go |
Deleted: entire Actions HTTP client and service interface |
github/actions/multi_client.go |
Deleted: multi-client caching layer |
github/actions/types.go |
Deleted: request/response types |
github/actions/errors.go |
Deleted: error types |
github/actions/sessionservice.go |
Deleted: SessionService interface |
github/actions/mocks_test.go |
Deleted: generated mocks |
github/actions/fake/client.go |
Deleted: fake client implementation |
github/actions/fake/multi_client.go |
Deleted: fake multi-client |
github/actions/testserver/server.go |
Deleted: test server helper (still referenced elsewhere) |
github/actions/testdata/* |
Deleted: test certificates (still referenced elsewhere) |
github/actions/*_test.go (multiple) |
Deleted: all tests for the removed code |
controllers/.../ephemeralrunnerset_controller.go |
Simplified error handling using scaleset errors, removed ActionsClient field |
controllers/.../ephemeralrunner_controller.go |
Simplified error handling using scaleset errors |
Comments suppressed due to low confidence (2)
github/actions/testserver/server.go:1
- The
github/actions/testserverpackage is deleted in this PR, but it is still imported and used in:
apis/actions.github.com/v1alpha1/tls_config_test.go(line 12)cmd/ghalistener/config/config_client_test.go(line 15)
These test files also reference github/actions/testdata files (certificates and keys) that are also being deleted. This will cause compilation failures in those test files.
github/actions/mocks_test.go:1
- The
.mockery.yamlstill referencesgithub.com/actions/actions-runner-controller/github/actionspackage withActionsServiceandSessionServiceinterfaces (lines 12-20), but both theActionsServiceinterface (fromclient.go) andSessionServiceinterface (fromsessionservice.go) have been deleted in this PR. The generated mock filemocks_test.gois also being deleted. This stale configuration will causemockeryto fail when run. These entries should be removed from.mockery.yaml.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
cae3aea to
052ac52
Compare
No description provided.