comments for K8 describe package #294

Philip-21 · 2023-04-10T20:45:44Z

Signed-off-by: Philip-21 [email protected]
Description
Created comments for k8 describe package

This PR fixes #287

Signed commits

Yes, I signed my commits.

Signed-off-by: Philip-21 <[email protected]>

Aisuko

Can we only include the code comments for describe.go? Before we confirm the scope of the mocking test, we could merge that test case

Philip-21 · 2023-04-11T06:25:44Z

@Aisuko from what you are saying, do you want me to only comment on the role for the describe.go so it could tally with describe_test.go and should i also take out some of the comments i wrote for the codes in the describe.go ?

Aisuko · 2023-04-11T06:53:08Z

@Aisuko from what you are saying, do you want me to only comment on the role for the describe.go so it could tally with describe_test.go and should I also take out some of the comments i wrote for the codes in the describe.go ?

Please keep the code comments in the descrble.go. The mock testing part already include in your previous PR right? This PR is only for code comments. So, we can merge it directly.

Aisuko · 2023-04-11T06:53:51Z

Please do not forget to update to the latest version code.

Philip-21 · 2023-04-11T08:59:27Z

@Aisuko from what you are saying, do you want me to only comment on the role for the describe.go so it could tally with describe_test.go and should I also take out some of the comments i wrote for the codes in the describe.go ?

Please keep the code comments in the descrble.go. The mock testing part already include in your previous PR right? This PR is only for code comments. So, we can merge it directly.

Yes this pr only includes code comments for the describe.go . The previous Pr is for the Mock testing

Philip-21 · 2023-04-11T09:03:07Z

Please do not forget to update to the latest version code.

Please how do you mean by this

Aisuko · 2023-04-12T01:49:00Z

Philip-21 · 2023-04-12T06:24:02Z

ok thanks, i have just updated branch to latest version.

Aisuko · 2023-04-12T07:03:36Z

We need to only include the describe.go file in this PR. This PR only aims to add code comment to describle.go

Signed-off-by: Philip Obiora <[email protected]>

Philip-21 · 2023-04-12T07:34:38Z

We need to only include the describe.go file in this PR. This PR only aims to add code comment to describle.go

ok i have removed the describe_test.go file . The describe.go file is what is in this pr

Aisuko

Thank you, please take a look and help me replace these code comments. We use code comment // in the file.

Aisuko · 2023-04-12T08:17:38Z

utils/kubernetes/describe/describe.go

 }

+/*


Replace to // DescribeType is an integer value that represents the Kubernetes resource that needs to be described.

Aisuko · 2023-04-12T08:18:00Z

utils/kubernetes/describe/describe.go

@@ -57,6 +64,12 @@ const (
 	EndpointSlice
 )

+/*


Replace to // The ResourceMap variable contains the GroupKind information of all the Kubernetes resources that can be described.

Aisuko · 2023-04-12T08:18:21Z

utils/kubernetes/describe/describe.go

@@ -88,22 +101,30 @@ var ResourceMap = map[DescribeType]schema.GroupKind{
 	EndpointSlice:             {Group: discoveryv1.GroupName, Kind: "EndpointSlice"},
 }

+/*


Replace to // The Describe() function takes a meshkitkube.Client object and a DescriberOptions object as input. // And it returns the description of the specified Kubernetes resource as a string.

Aisuko · 2023-04-12T08:18:39Z

utils/kubernetes/describe/describe.go

 	output, err := describer.Describe(options.Namespace, options.Name, describerSetting)
 	if err != nil {
 		return "", err
 	}
-
+	/*


Please remove this code comment.

Ok, i have made the changes

making adjustments to comments Signed-off-by: Philip Obiora <[email protected]>

Aisuko

After remove these blanks and pass CI. We can merge it. You can get more detail from https://stackoverflow.com/questions/69049182/file-is-not-gofmt-ed-with-s-why-is-this-happening-and-how-to-resolve-it

Aisuko · 2023-04-13T03:12:13Z

utils/kubernetes/describe/describe.go

 }

+ // DescribeType is an integer value that represents the Kubernetes resource that needs to be described.


Here is a blank at the beginning, CI cannot run succeed. So, please remove it.

Aisuko · 2023-04-13T03:12:30Z

utils/kubernetes/describe/describe.go

@@ -57,6 +60,7 @@ const (
 	EndpointSlice
 )

+ // The ResourceMap variable contains the GroupKind information of all the Kubernetes resources that can be described.


Aisuko · 2023-04-13T03:12:38Z

utils/kubernetes/describe/describe.go

@@ -88,22 +92,23 @@ var ResourceMap = map[DescribeType]schema.GroupKind{
 	EndpointSlice:             {Group: discoveryv1.GroupName, Kind: "EndpointSlice"},
 }

+// The Describe() function takes a meshkitkube.Client object and a DescriberOptions object as input.
+ // And it returns the description of the specified Kubernetes resource as a string.


You want me to remove the comments or the spaces in between the comments ?

The space at the second comment

i have removed the spaces

Signed-off-by: Philip Obiora <[email protected]>

Aisuko

LGTM

Signed-off-by: Philip Obiora <[email protected]>

Aisuko · 2023-04-13T08:28:06Z

CI may have some issues, I check the PR manually by using golangci-lint, there is no error.

ErrGetDescriberFunc was defined under the same package.

Aisuko

LGTM

welcome · 2023-04-13T08:28:54Z

Thanks for your contribution to the Layer5 community! 🎉

Philip-21 · 2023-04-13T08:31:18Z

CI may have some issues, I check the PR manually by using golangci-lint, there is no error. ErrGetDescriberFunc was defined under the same package.

does it mean that it shouldn't be there .

Aisuko · 2023-04-13T08:38:18Z

CI may have some issues, I check the PR manually by using golangci-lint, there is no error. ErrGetDescriberFunc was defined under the same package.

does it mean that it shouldn't be there .

No worries, this is not related to this PR.

Philip-21 · 2023-04-13T08:51:36Z

CI may have some issues, I check the PR manually by using golangci-lint, there is no error. ErrGetDescriberFunc was defined under the same package.

does it mean that it shouldn't be there .

No worries, this is not related to this PR.

Ok I would start working on the describe_test in the other pr and make changes where necessary.
And I will Write more unit test for the K8 packages .

Philip-21 added 3 commits April 10, 2023 09:21

created unit test for the k8 describe package

38d4915

Signed-off-by: Philip-21 <[email protected]>

Added signoff DCO

6e9756d

Signed-off-by: Philip-21 <[email protected]>

created comments for the k8 describe.go file

66f0273

Signed-off-by: Philip-21 <[email protected]>

Philip-21 changed the title ~~K8 describe package~~ comments for K8 describe package Apr 10, 2023

Aisuko self-requested a review April 11, 2023 01:01

Aisuko requested changes Apr 11, 2023

View reviewed changes

Merge branch 'master' into k8-describe-package

a085bf3

Delete describe_test.go

3b612b3

Signed-off-by: Philip Obiora <[email protected]>

Aisuko self-requested a review April 12, 2023 07:57

Aisuko self-assigned this Apr 12, 2023

Aisuko requested changes Apr 12, 2023

View reviewed changes

Aisuko added kind/enhancement Improvement in current feature language/go Golang related labels Apr 12, 2023

adjusting comments

4a4a055

making adjustments to comments Signed-off-by: Philip Obiora <[email protected]>

Aisuko requested changes Apr 13, 2023

View reviewed changes

removing spaces

3bdd184

Signed-off-by: Philip Obiora <[email protected]>

Aisuko approved these changes Apr 13, 2023

View reviewed changes

adjusting spaces

c59ad50

Signed-off-by: Philip Obiora <[email protected]>

Aisuko self-requested a review April 13, 2023 08:28

Aisuko approved these changes Apr 13, 2023

View reviewed changes

Aisuko merged commit 49c5bc1 into meshery:master Apr 13, 2023

Philip-21 deleted the k8-describe-package branch April 14, 2023 18:50

Philip-21 restored the k8-describe-package branch April 14, 2023 18:50

Philip-21 deleted the k8-describe-package branch April 14, 2023 18:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

comments for K8 describe package #294

comments for K8 describe package #294

Philip-21 commented Apr 10, 2023 •

edited

Loading

Aisuko left a comment

Philip-21 commented Apr 11, 2023 •

edited

Loading

Aisuko commented Apr 11, 2023

Aisuko commented Apr 11, 2023

Philip-21 commented Apr 11, 2023 •

edited

Loading

Philip-21 commented Apr 11, 2023

Aisuko commented Apr 12, 2023

Philip-21 commented Apr 12, 2023

Aisuko commented Apr 12, 2023

Philip-21 commented Apr 12, 2023 •

edited

Loading

Aisuko left a comment

Aisuko Apr 12, 2023

Aisuko Apr 12, 2023

Aisuko Apr 12, 2023

Aisuko Apr 12, 2023

Philip-21 Apr 12, 2023

Aisuko left a comment •

edited

Loading

Aisuko Apr 13, 2023

Aisuko Apr 13, 2023

Aisuko Apr 13, 2023

Philip-21 Apr 13, 2023

Aisuko Apr 13, 2023

Philip-21 Apr 13, 2023

Aisuko left a comment

Aisuko commented Apr 13, 2023 •

edited

Loading

Aisuko left a comment

welcome bot commented Apr 13, 2023

Philip-21 commented Apr 13, 2023

Aisuko commented Apr 13, 2023 •

edited

Loading

Philip-21 commented Apr 13, 2023

		}

		// DescribeType is an integer value that represents the Kubernetes resource that needs to be described.

comments for K8 describe package #294

comments for K8 describe package #294

Conversation

Philip-21 commented Apr 10, 2023 • edited Loading

Aisuko left a comment

Choose a reason for hiding this comment

Philip-21 commented Apr 11, 2023 • edited Loading

Aisuko commented Apr 11, 2023

Aisuko commented Apr 11, 2023

Philip-21 commented Apr 11, 2023 • edited Loading

Philip-21 commented Apr 11, 2023

Aisuko commented Apr 12, 2023

Philip-21 commented Apr 12, 2023

Aisuko commented Apr 12, 2023

Philip-21 commented Apr 12, 2023 • edited Loading

Aisuko left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Aisuko left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Aisuko left a comment

Choose a reason for hiding this comment

Aisuko commented Apr 13, 2023 • edited Loading

Aisuko left a comment

Choose a reason for hiding this comment

welcome bot commented Apr 13, 2023

Philip-21 commented Apr 13, 2023

Aisuko commented Apr 13, 2023 • edited Loading

Philip-21 commented Apr 13, 2023

Philip-21 commented Apr 10, 2023 •

edited

Loading

Philip-21 commented Apr 11, 2023 •

edited

Loading

Philip-21 commented Apr 11, 2023 •

edited

Loading

Philip-21 commented Apr 12, 2023 •

edited

Loading

Aisuko left a comment •

edited

Loading

Aisuko commented Apr 13, 2023 •

edited

Loading

Aisuko commented Apr 13, 2023 •

edited

Loading