GoogleCloudPlatform / k8s-cloud-provider

Support code for implementing a Kubernetes cloud provider for Google Cloud Platform
Apache License 2.0
37 stars 46 forks source link

Move functions using global test flags to *_test file #188

Closed AwesomePatrol closed 5 months ago

AwesomePatrol commented 5 months ago

Newly added helper functions were part of the package while the variables they depend on were defined in the *_test.go resulting in compile failures.

Alternative solutions:

  1. Move test-helpers to helpers_test - all functions/variables in this file are unexported so it is useless as a package anyway
  2. Add new arguments to newly added functions so they won't depend on global variables - helpers would be not that helpful and it would be better to inline them (as each is called only once)

/cc @kl52752 /assign @bowei

kl52752 commented 5 months ago

thanks for fixing the build. /lgtm

bowei commented 5 months ago

How did we manage to commit a build break? Can we (separate PR) fix the build so this is not possible to submit?

bowei commented 5 months ago

/lgtm /approve

google-oss-prow[bot] commented 5 months ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: AwesomePatrol, bowei

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/GoogleCloudPlatform/k8s-cloud-provider/blob/master/OWNERS)~~ [bowei] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
kl52752 commented 5 months ago

How did we manage to commit a build break? Can we (separate PR) fix the build so this is not possible to submit?

Prow is doing make test which is not building the repo. I'm fixing that.

kl52752 commented 5 months ago

@bowei Fix PR PTAL