Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove unused flag #1913

Merged
merged 1 commit into from
Sep 26, 2019
Merged

Remove unused flag #1913

merged 1 commit into from
Sep 26, 2019

Conversation

nrb
Copy link
Contributor

@nrb nrb commented Sep 26, 2019

Quick followup to #1906

Signed-off-by: Nolan Brubaker [email protected]

Signed-off-by: Nolan Brubaker <[email protected]>
@nrb nrb requested review from carlisia and skriss September 26, 2019 18:37
Copy link
Contributor

@skriss skriss left a comment

Choose a reason for hiding this comment

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

LGTM

@skriss skriss merged commit a1545b7 into vmware-tanzu:master Sep 26, 2019
jessestuart added a commit to jessestuart/velero that referenced this pull request Sep 28, 2019
* upstream/master: (38 commits)
  sync controller: replace revision file with full diff each interval (vmware-tanzu#1892)
  Increment logging for item backupper (vmware-tanzu#1904)
  Add LD_LIBRARY_PATH as an env varible for the use of vsphere plugin (vmware-tanzu#1893)
  Remove unused flag (vmware-tanzu#1913)
  Use layers in the builder Dockerfile (vmware-tanzu#1907)
  Fix for vmware-tanzu#1888: check item's original namespace, not remapped one, for inclusion/exclusion (vmware-tanzu#1909)
  fail on make verify if generated CRDs differ (vmware-tanzu#1906)
  velero API type changes for structural schema CRDs (vmware-tanzu#1898)
  Generate CRDs with structural schema (vmware-tanzu#1885)
  Plan for moving plugin repos (vmware-tanzu#1870)
  move plugin proto updating into make update (vmware-tanzu#1887)
  Add features package (vmware-tanzu#1849)
  GCP: support specifying Cloud KMS key name for backup storage locations (vmware-tanzu#1879)
  Adds to website (vmware-tanzu#1882)
  proposal for generating Velero CRDs with structural schema (vmware-tanzu#1875)
  Improve contributing docs (vmware-tanzu#1852)
  [doc] Diagram (image) now mentions velero  (vmware-tanzu#1877)
  AWS: add support for arbitrary SSE algorithms, e.g. AES256 (vmware-tanzu#1869)
  update restic docs for PR vmware-tanzu#1807 (vmware-tanzu#1867)
  changelog for PR vmware-tanzu#1864
  ...
@prydonius
Copy link
Contributor

Wasn't this flag being used by generate-groups.sh:

?

@nrb
Copy link
Contributor Author

nrb commented Oct 4, 2019

Ahh, was it? That makes sense, I didn't see that the full args were passed on there.

@prydonius
Copy link
Contributor

I believe it's being used there, I didn't add the flag as part of #1885, I just used it to skip the CRD generation (https://github.com/vmware-tanzu/velero/pull/1885/files#diff-892d90adfc5482f1fee4517a4ad27aeeR44). I'll verify that it's being used be generate-groups.sh, and if so, we can revert this.

@skriss
Copy link
Contributor

skriss commented Oct 4, 2019

Oops, missed that in review too

@prydonius
Copy link
Contributor

Can confirm that the --verify-only flag is needed here.

After making an API change (e.g. changing BackupStatus CompletionTimestamp field to be a *metav1.Time type), running make verify does not fail and pkg/apis/velero/v1/zz_generated.deepcopy.go gets modified. With the --verify-only flag added back, make verify fails as expected and pkg/apis/velero/v1/zz_generated.deepcopy.go is unchanged:

Running all verification scripts
Verifying gofmt
Verifying gofmt - done!
Verifying goimports
Verifying goimports - done!
+ [[ -z /go ]]
+ [[ ! -d /go/src/k8s.io/code-generator ]]
+ [[ ! -d /go/src/sigs.k8s.io/controller-tools ]]
+ /go/src/k8s.io/code-generator/generate-groups.sh all github.com/vmware-tanzu/velero/pkg/generated github.com/vmware-tanzu/velero/pkg/apis velero:v1 --go-header-file /go/src/github.com/vmware-tanzu/velero/hack/boilerplate.go.txt --verify-only
Generating deepcopy funcs
F1004 23:15:43.305762    3876 main.go:82] Error: Failed executing generator: some packages had errors:
errors in package "github.com/vmware-tanzu/velero/pkg/apis/velero/v1":
output for "v1/zz_generated.deepcopy.go" differs; first existing/expected diff:
  "n.CompletionTimestamp.DeepCopyInto(&out.CompletionTimestamp)\n\treturn\n}\n\n// DeepCopy is an autogenera"
  "f in.CompletionTimestamp != nil {\n\t\tin, out := &in.CompletionTimestamp, &out.CompletionTimestamp\n\t\t*"

make[1]: *** [shell] Error 255
make: *** [verify] Error 2

I'll open a PR to revert this change.

prydonius added a commit that referenced this pull request Oct 4, 2019
prydonius added a commit to prydonius/velero that referenced this pull request Oct 4, 2019
This reverts commit a1545b7.

Signed-off-by: Adnan Abdulhussein <[email protected]>
skriss pushed a commit that referenced this pull request Oct 7, 2019
This reverts commit a1545b7.

Signed-off-by: Adnan Abdulhussein <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants