Skip to content
This repository has been archived by the owner on Sep 12, 2023. It is now read-only.

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

Merged
merged 1 commit into from
Feb 24, 2023

Conversation

Syulin7
Copy link
Contributor

@Syulin7 Syulin7 commented Feb 9, 2023

Set None as default.

Fixes: kubeflow/training-operator#1753

@johnugeorge
Copy link
Member

/lgtm

@Syulin7
Copy link
Contributor Author

Syulin7 commented Feb 13, 2023

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
Copy link
Member

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

curl -sfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin

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

@Syulin7
Copy link
Contributor Author

Syulin7 commented Feb 13, 2023

OK, I'll give it a try.

…int staticcheck.

Signed-off-by: Syulin7 <735122171@qq.com>
@Syulin7
Copy link
Contributor Author

Syulin7 commented Feb 13, 2023

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

@tenzen-y
Copy link
Member

@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
Copy link
Contributor Author

Syulin7 commented Feb 20, 2023

@johnugeorge @terrytangyuan PTAL, thanks.

@johnugeorge
Copy link
Member

Thanks @Syulin7

/lgtm
/approve

@google-oss-prow google-oss-prow bot added the lgtm label Feb 20, 2023
@Syulin7
Copy link
Contributor Author

Syulin7 commented Feb 21, 2023

@terrytangyuan @gaocegege @Jeffwan PTAL, thanks.

@@ -179,7 +179,7 @@ const (
// active.
type RunPolicy struct {
// CleanPodPolicy defines the policy to kill pods after the job completes.
// Default to Running.
// Default to None.
Copy link
Member

Choose a reason for hiding this comment

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

No code changes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the code changes in training operator, PR: kubeflow/training-operator#1754. and there are only comments in the common library. The generation of the CRD in the training operator will use it.

@terrytangyuan
Copy link
Member

/lgtm

@johnugeorge
Copy link
Member

@terrytangyuan Can you merge it as well?

Copy link
Member

@terrytangyuan terrytangyuan left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@google-oss-prow
Copy link

[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:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@google-oss-prow google-oss-prow bot merged commit 4dbf18c into kubeflow:master Feb 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The default value for CleanPodPolicy is inconsistent.
4 participants