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.

Sign up for GitHub

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

Jump to bottom

Filter resources by label to catch aggregated api mistakes #198

Merged

cppforlife merged 5 commits into develop from labels

Mar 5, 2021

Contributor

gcheadle-vmware commented Mar 4, 2021

No description provided.


          Add Resources interface

2ab6fdd

- Change Resources type to Impl

vmwclabot added the cla-not-required label


          Filter resources with label selector to catch mistakes by an aggregat…

e4262f2

…ed api (#160)

Signed-off-by: Garrett Cheadle <gcheadle@vmware.com>

gcheadle-vmware force-pushed the labels branch from 82580ed to e4262f2 Compare

March 4, 2021 02:03

joaopapereira reviewed

View reviewed changes

Member

joaopapereira left a comment

Added some comments on the PR. The implementation looks good, more some nitpicks.
Also not 100% convinced on the usage of testify/mock.

pkg/kapp/resources/resources.go Outdated Show resolved Hide resolved

pkg/kapp/resources/resources.go Outdated Show resolved Hide resolved

pkg/kapp/resources/identified_resources_list_test.go Show resolved Hide resolved

cppforlife reviewed

View reviewed changes

pkg/kapp/resources/identified_resources_list.go Outdated Show resolved Hide resolved

cppforlife reviewed

View reviewed changes

pkg/kapp/resources/identified_resources_list_test.go Outdated Show resolved Hide resolved

cppforlife reviewed

View reviewed changes

pkg/kapp/resources/identified_resources_list_test.go Show resolved Hide resolved

cppforlife reviewed

View reviewed changes

pkg/kapp/resources/identified_resources_list_test.go Outdated Show resolved Hide resolved

cppforlife reviewed

View reviewed changes

pkg/kapp/resources/identified_resources_list_test.go Outdated Show resolved Hide resolved

cppforlife reviewed

View reviewed changes

pkg/kapp/resources/identified_resources_list_test.go Outdated Show resolved Hide resolved

cppforlife reviewed

View reviewed changes

pkg/kapp/resources/identified_resources_list_test.go Outdated Show resolved Hide resolved

cppforlife reviewed

View reviewed changes

pkg/kapp/resources/identified_resources_list_test.go Outdated Show resolved Hide resolved

cppforlife reviewed

View reviewed changes

pkg/kapp/resources/identified_resources_list_test.go Outdated Show resolved Hide resolved

cppforlife reviewed

View reviewed changes

pkg/kapp/resources/identified_resources_list_test.go Outdated Show resolved Hide resolved

cppforlife reviewed

View reviewed changes

pkg/kapp/resources/identified_resources_list_test.go Outdated Show resolved Hide resolved

gcheadle-vmware added 2 commits

March 4, 2021 16:08


          Remove mocks and update test

91b61b0

* renamed Impl -> ResourcesImpl
* update comment about aggregated APIs


          Add exclude rule to golangci-lint

b1ca53e

* ignores stuttering linting error

gcheadle-vmware force-pushed the labels branch from ec85a83 to b1ca53e Compare

March 5, 2021 00:09

joaopapereira reviewed

View reviewed changes

Member

joaopapereira left a comment

Just a minor comment on the test location

pkg/kapp/resources/identified_resources_list_test.go Outdated Show resolved Hide resolved


          Update test package to resources_test

7168dfc

cppforlife merged commit 7a4f630 into develop

cppforlife deleted the labels branch

March 5, 2021 19:52

gcheadle-vmware mentioned this pull request

WIP: dumps resources that do not have labels #176

Closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-not-required