Followup Antctl Tranform Code Move #6062
Conversation
| var npList = &cpv1beta.NetworkPolicyList{ | ||
| Items: []cpv1beta.NetworkPolicy{npA, npC, npB}, | ||
| } | ||
|
|
||
| var npListK8s = &cpv1beta.NetworkPolicyList{ |
There was a problem hiding this comment.
I am not sure I understand the naming here, why is one list called npList and the other npListK8s, as opposed to npList1 and npList2? What does the K8s suffix stand for?
There was a problem hiding this comment.
I was thinking npList2 has K8s NPs to test the sort, which npList1 hasn't. I have indicated this difference in the test case names instead, and changed to npList1 and npList2.
| // NewNetworkPolicyEvaluation transforms the arguments in the formats defined by antctl | ||
| // usages, to NetworkPolicyEvaluation format required for client resource request. |
There was a problem hiding this comment.
nit: I'd suggest the following instead
NewNetworkPolicyEvaluation creates a new NetworkPolicyEvaluation resource request from the command-line arguments provided to antctl.
| @@ -12,7 +12,7 @@ | |||
| // See the License for the specific language governing permissions and | |||
| // limitations under the License. | |||
|
|
|||
| package parameter | |||
| package networkpolicy | |||
There was a problem hiding this comment.
Should the file name be request.go given this is already under transform package?
There was a problem hiding this comment.
Updated the name. I noticed that all the files under transform/* package are named transform.go, does it matter?
There was a problem hiding this comment.
you could rename all the other files response.go if you wanted to be consistent
There was a problem hiding this comment.
I noticed that all the files under
transform/*package are namedtransform.go, does it matter?
It doesn't matter that a file has the name as its package. It's actually quite common when there is only 1 file in the package. Normally the file having the same name as the package is considered the "main" file containing the core interface, struct and functions, and other files normally contain structs and functions assisting the core ones. It would be redundant to prepend the package name to all these files.
Unifying other packages to reponse.go also sounds good.
qiyueyao
force-pushed
the
antctl-followup
branch
from
7317eb0
to
f275d81
Compare
This solution refactors the transform functions for antctl networkpolicy to indicate request and response. Also adds some additional unit tests. Signed-off-by: Qiyue Yao <yaoq@vmware.com>
qiyueyao
force-pushed
the
antctl-followup
branch
from
f275d81
to
c9e3c4b
Compare
|
/skip-all |
This solution refactors the transform functions for
antctl networkpolicyto the same place, but indicates request and response difference. (base on this discussion)Also adds some additional unit tests.