kubeflow / common

Common APIs and libraries shared by other Kubeflow operator repositories.
Apache License 2.0
51 stars 73 forks source link

Change the default value of CleanPodPolicy to None in comments. #210

Closed Syulin7 closed 1 year ago

Syulin7 commented 1 year ago

Set None as default.

Fixes: https://github.com/kubeflow/training-operator/issues/1753

johnugeorge commented 1 year ago

/lgtm

Syulin7 commented 1 year ago

build/test was failed:

Run golangci-lint run --config=linter_config.yaml ./...
level=warning msg="[runner] The linter 'deadcode' is deprecated (since v1.49.0) due to: The owner seems to have abandoned the linter.  Replaced by unused."
level=warning msg="[runner] The linter 'varcheck' is deprecated (since v1.49.0) due to: The owner seems to have abandoned the linter.  Replaced by unused."
Error: pkg/controller.v1/common/job_test.go:194:2: SA4017: AddDate doesn't have side effects and its return value is ignored (staticcheck)
    oneDayAgo.AddDate(0, 0, -1)
    ^
Error: Process completed with exit code 1.

@johnugeorge @tenzen-y Do you know the reason why it happened, or can you help make it retry.

tenzen-y commented 1 year ago

I guess the behavior of staticcheck changed since we always install the latest golangci-lint like the following:

https://github.com/kubeflow/common/blob/9ec55d141f90faaf52fd6df271e987e5a6781945/.github/workflows/build.yml#L42

So we need to fix the lint error. Can you fix the error?

Syulin7 commented 1 year ago

OK, I'll give it a try.

Syulin7 commented 1 year ago

@tenzen-y I fixed the error, can you help make it retry.

tenzen-y commented 1 year ago

@tenzen-y I fixed the error, can you help make it retry.

Thanks for fixing the error. However, I don't have permission to approve the CI.

@kubeflow/wg-training-leads Can you approve CI?

Syulin7 commented 1 year ago

@johnugeorge @terrytangyuan PTAL, thanks.

johnugeorge commented 1 year ago

Thanks @Syulin7

/lgtm /approve

Syulin7 commented 1 year ago

@terrytangyuan @gaocegege @Jeffwan PTAL, thanks.

terrytangyuan commented 1 year ago

/lgtm

johnugeorge commented 1 year ago

@terrytangyuan Can you merge it as well?

google-oss-prow[bot] commented 1 year ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: johnugeorge, terrytangyuan

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/kubeflow/common/blob/master/OWNERS)~~ [terrytangyuan] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment