Add verify script for deep-copies generator #545

pravarag · 2021-04-08T06:20:04Z

This PR is related to: #483

This PR contains changes for verifying deep-copies generator. Below are the logs,

When there are no changes,

-bash>descheduler(add-verify-script-deep-copies)$
 ->  make verify-gen
./hack/verify-conversions.sh
Generated conversions verified.
./hack/verify-deep-copies.sh
Generated deep-copies verified.

After making a change in the api similar to the change mentioned here for validating failed scenario,

-bash>descheduler(add-verify-script-deep-copies)$
 ->  ./hack/verify-deep-copies.sh
Generated deep-copies output differs:
diff -Naupr pkg/api/v1alpha1/zz_generated.deepcopy.go ./hack/../_tmp/kube-verify.whSrQp/descheduler/pkg/api/v1alpha1/zz_generated.deepcopy.go
--- pkg/api/v1alpha1/zz_generated.deepcopy.go	2021-04-08 11:16:08.000000000 +0530
+++ ./hack/../_tmp/kube-verify.whSrQp/descheduler/pkg/api/v1alpha1/zz_generated.deepcopy.go	2021-04-08 11:17:03.000000000 +0530
@@ -61,7 +61,6 @@ func (in *DeschedulerPolicy) DeepCopyInt
 		*out = new(int)
 		**out = **in
 	}
-	out.Foo = in.Foo
 	return
 }

Generated deep-copies verify failed.

k8s-ci-robot · 2021-04-08T06:20:11Z

Hi @pravarag. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

seanmalloy · 2021-04-08T12:27:19Z

This PR fixes: #483

Please edit the description to say "This PR is related to #483".

seanmalloy · 2021-04-08T12:29:20Z

hack/verify-deep-copies.sh

+fi
+popd > /dev/null 2>&1
+
+echo "Generated deep-copies verified."


Please delete this line. You can also remove this line from the verify-conversions.sh script too.

Are you referring to, line 34 or line 36 ?

Some kind of "success" message is helpful rather than printing nothing, to show that the script did actually complete (for local runs, and for CI where we are calling multiple scripts).

I would also update the failure message to include a Please run ./hack/update-deep-copies.sh (same for conversions if it doesn't already)

Updated with latest push.

Okay, I'll just put back those in with the updated failure message as well.

Yeah, sorry for the back and forth but I do think these are worth having... You may want to wait for @seanmalloy to see if he agrees or there is something I'm overlooking 🙂

It's fine to keep the echo statement.

@pravarag you can re-add the echo statement. Sorry for the confusion.

seanmalloy · 2021-04-08T12:30:33Z

/ok-to-test

seanmalloy · 2021-04-08T15:18:15Z

hack/verify-conversions.sh

@@ -32,5 +32,3 @@ if ! _out="$(diff -Naupr pkg/ "${_deschedulertmp}/pkg/")"; then
    exit 1
 fi
 popd > /dev/null 2>&1
-
-echo "Generated conversions verified."


Please don't delete these lines. Sorry for the confusion.

pravarag · 2021-04-08T17:22:41Z

I've added back the echo statements and updated the failure message as well.

damemi · 2021-04-08T18:00:03Z

@pravarag thanks, this looks good to me
/approve

seanmalloy · 2021-04-09T04:19:17Z

/lgtm
/kind cleanup

k8s-ci-robot · 2021-04-09T04:19:41Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: damemi, pravarag, seanmalloy

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~~ [damemi,seanmalloy]

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

pravarag · 2021-04-09T04:36:48Z

Thanks @seanmalloy @damemi for merging this 🙂

…t-deep-copies Add verify script for deep-copies generator

k8s-ci-robot requested review from k82cn and seanmalloy April 8, 2021 06:20

k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Apr 8, 2021

seanmalloy requested changes Apr 8, 2021

View reviewed changes

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Apr 8, 2021

pravarag force-pushed the add-verify-script-deep-copies branch from 58b2314 to 487a3e9 Compare April 8, 2021 14:54

seanmalloy reviewed Apr 8, 2021

View reviewed changes

add verify script for deep-copies gen

4fce2ca

pravarag force-pushed the add-verify-script-deep-copies branch from 487a3e9 to 4fce2ca Compare April 8, 2021 17:21

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 8, 2021

k8s-ci-robot added the kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. label Apr 9, 2021

k8s-ci-robot assigned seanmalloy Apr 9, 2021

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 9, 2021

seanmalloy approved these changes Apr 9, 2021

View reviewed changes

k8s-ci-robot merged commit 9fe585c into kubernetes-sigs:master Apr 9, 2021

damemi mentioned this pull request May 17, 2021

Add verify script for govet & fix pkg/descheduler/descheduler_test.go #568

Merged

briend pushed a commit to briend/descheduler that referenced this pull request Feb 11, 2022

Merge pull request kubernetes-sigs#545 from pravarag/add-verify-scrip…

9f5f71b

…t-deep-copies Add verify script for deep-copies generator

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add verify script for deep-copies generator #545

Add verify script for deep-copies generator #545

pravarag commented Apr 8, 2021 •

edited

Loading

k8s-ci-robot commented Apr 8, 2021

seanmalloy commented Apr 8, 2021

seanmalloy Apr 8, 2021 •

edited

Loading

pravarag Apr 8, 2021

seanmalloy Apr 8, 2021

damemi Apr 8, 2021 •

edited

Loading

pravarag Apr 8, 2021

pravarag Apr 8, 2021

damemi Apr 8, 2021

seanmalloy Apr 8, 2021 •

edited

Loading

seanmalloy Apr 8, 2021

seanmalloy commented Apr 8, 2021

seanmalloy Apr 8, 2021

pravarag commented Apr 8, 2021

damemi commented Apr 8, 2021

seanmalloy commented Apr 9, 2021

k8s-ci-robot commented Apr 9, 2021

pravarag commented Apr 9, 2021

Add verify script for deep-copies generator #545

Add verify script for deep-copies generator #545

Conversation

pravarag commented Apr 8, 2021 • edited Loading

k8s-ci-robot commented Apr 8, 2021

seanmalloy commented Apr 8, 2021

seanmalloy Apr 8, 2021 • edited Loading

Choose a reason for hiding this comment

pravarag Apr 8, 2021

Choose a reason for hiding this comment

seanmalloy Apr 8, 2021

Choose a reason for hiding this comment

damemi Apr 8, 2021 • edited Loading

Choose a reason for hiding this comment

pravarag Apr 8, 2021

Choose a reason for hiding this comment

pravarag Apr 8, 2021

Choose a reason for hiding this comment

damemi Apr 8, 2021

Choose a reason for hiding this comment

seanmalloy Apr 8, 2021 • edited Loading

Choose a reason for hiding this comment

seanmalloy Apr 8, 2021

Choose a reason for hiding this comment

seanmalloy commented Apr 8, 2021

seanmalloy Apr 8, 2021

Choose a reason for hiding this comment

pravarag commented Apr 8, 2021

damemi commented Apr 8, 2021

seanmalloy commented Apr 9, 2021

k8s-ci-robot commented Apr 9, 2021

pravarag commented Apr 9, 2021

pravarag commented Apr 8, 2021 •

edited

Loading

seanmalloy Apr 8, 2021 •

edited

Loading

damemi Apr 8, 2021 •

edited

Loading

seanmalloy Apr 8, 2021 •

edited

Loading