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

Adding comments on Kanister Resource Types Fields in types.go #1751

Merged
merged 37 commits into from
Jan 9, 2023

Conversation

Kartik-Garg
Copy link
Contributor

@Kartik-Garg Kartik-Garg commented Nov 21, 2022

There were comments missing from kanister resource types fields in types.go ,because of which there was missing description while creating the documentation using tools like https://github.com/ahmetb/gen-crd-api-reference-docs/ . So, added missing comments on fields present in types.go

Signed-off-by: Kartik-Garg kartikgarg938@gmail.com

Change Overview

The documentation generated now has description of all the fields present in Types.go:
Screenshot from 2022-11-21 20-49-36

Added missing comments on fields on types.go file.

Pull request type

Please check the type of change your PR introduces:

  • 🚧 Work in Progress
  • 🌈 Refactoring (no functional changes, no api changes)
  • 🐹 Trivial/Minor
  • 🐛 Bugfix
  • 🌻 Feature
  • 🗺️ Documentation
  • 🤖 Test

Issues

Test Plan

  • 💪 Manual
  • ⚡ Unit test
  • 💚 E2E

There were comments missing from kanister resource types fields in types.go which because of which the documentation was not coming out to be proper. So, added missing comments on types.go

Signed-off-by: Kartik-Garg <kartikgarg938@gmail.com>
Copy link
Contributor

@PrasadG193 PrasadG193 left a comment

Choose a reason for hiding this comment

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

@Kartik-Garg Please go through https://docs.kanister.io/architecture.html to understand the significance of each field and correct the comments. I went through few but will re-review once you correct the field comments by referring the doc

pkg/apis/cr/v1alpha1/types.go Outdated Show resolved Hide resolved
pkg/apis/cr/v1alpha1/types.go Outdated Show resolved Hide resolved
pkg/apis/cr/v1alpha1/types.go Outdated Show resolved Hide resolved
pkg/apis/cr/v1alpha1/types.go Outdated Show resolved Hide resolved
pkg/apis/cr/v1alpha1/types.go Outdated Show resolved Hide resolved
pkg/apis/cr/v1alpha1/types.go Outdated Show resolved Hide resolved
pkg/apis/cr/v1alpha1/types.go Outdated Show resolved Hide resolved
pkg/apis/cr/v1alpha1/types.go Outdated Show resolved Hide resolved
pkg/apis/cr/v1alpha1/types.go Outdated Show resolved Hide resolved
pkg/apis/cr/v1alpha1/types.go Outdated Show resolved Hide resolved
@Kartik-Garg
Copy link
Contributor Author

Kartik-Garg commented Nov 30, 2022

@Kartik-Garg Please go through https://docs.kanister.io/architecture.html to understand the significance of each field and correct the comments. I went through few but will re-review once you correct the field comments by referring the doc

@PrasadG193 I see, I did not realize we could write comments by referring to https://docs.kanister.io/architecture.html
I had written comments with what I thought was right :p
Thanks a lot for pointing it out :D

pkg/apis/cr/v1alpha1/types.go Outdated Show resolved Hide resolved
pkg/apis/cr/v1alpha1/types.go Outdated Show resolved Hide resolved
pkg/apis/cr/v1alpha1/types.go Outdated Show resolved Hide resolved
pkg/apis/cr/v1alpha1/types.go Outdated Show resolved Hide resolved
pkg/apis/cr/v1alpha1/types.go Outdated Show resolved Hide resolved
pkg/apis/cr/v1alpha1/types.go Outdated Show resolved Hide resolved
pkg/apis/cr/v1alpha1/types.go Outdated Show resolved Hide resolved
pkg/apis/cr/v1alpha1/types.go Outdated Show resolved Hide resolved
pkg/apis/cr/v1alpha1/types.go Outdated Show resolved Hide resolved
pkg/apis/cr/v1alpha1/types.go Outdated Show resolved Hide resolved
Kartik-Garg and others added 5 commits January 3, 2023 03:39
Added new comments and modified few existing comments for better understanding.

Signed-off-by: Kartik-Garg <kartikgarg938@gmail.com>
Added new comments and modified existing comments for better understanding.

Signed-off-by: Kartik-Garg <kartikgarg938@gmail.com>
…k-Garg/kanister into commentsOnKanisterResource

Signed-off-by: Kartik-Garg <kartikgarg938@gmail.com>
Added new comments and modified few existing comments present in types.go for better understanding.

Signed-off-by: Kartik-Garg <kartikgarg938@gmail.com>
@viveksinghggits
Copy link
Contributor

@Kartik-Garg have you addressed all the review comment's suggestion in this PR? If yes, can you please mark the comments resolved.

@Kartik-Garg
Copy link
Contributor Author

Kartik-Garg commented Jan 3, 2023

@Kartik-Garg have you addressed all the review comment's suggestion in this PR? If yes, can you please mark the comments resolved.

@viveksinghggits There's just one comment left about Prefix , other than that I have made changes and marked the comments as resolved.

Modified few comments present in the types.go file so it would be easier to understand the fields.

Signed-off-by: Kartik-Garg <kartikgarg938@gmail.com>
Copy link
Contributor

@pavannd1 pavannd1 left a comment

Choose a reason for hiding this comment

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

Part 1 of my review. I'll post the remaining comments tomorrow

pkg/apis/cr/v1alpha1/types.go Outdated Show resolved Hide resolved
pkg/apis/cr/v1alpha1/types.go Outdated Show resolved Hide resolved
pkg/apis/cr/v1alpha1/types.go Outdated Show resolved Hide resolved
pkg/apis/cr/v1alpha1/types.go Outdated Show resolved Hide resolved
pkg/apis/cr/v1alpha1/types.go Outdated Show resolved Hide resolved
pkg/apis/cr/v1alpha1/types.go Outdated Show resolved Hide resolved
pkg/apis/cr/v1alpha1/types.go Outdated Show resolved Hide resolved
pkg/apis/cr/v1alpha1/types.go Outdated Show resolved Hide resolved
pkg/apis/cr/v1alpha1/types.go Outdated Show resolved Hide resolved
pkg/apis/cr/v1alpha1/types.go Outdated Show resolved Hide resolved
Made modification on the comments in the types.go file

Signed-off-by: Kartik-Garg <kartikgarg938@gmail.com>
@Kartik-Garg
Copy link
Contributor Author

Part 1 of my review. I'll post the remaining comments tomorrow

Thanks a lot for the reviews, made changes according to the review comments, eagerly waiting for upcoming parts :D

pkg/apis/cr/v1alpha1/types.go Outdated Show resolved Hide resolved
pkg/apis/cr/v1alpha1/types.go Outdated Show resolved Hide resolved
pkg/apis/cr/v1alpha1/types.go Outdated Show resolved Hide resolved
pkg/apis/cr/v1alpha1/types.go Outdated Show resolved Hide resolved
pkg/apis/cr/v1alpha1/types.go Outdated Show resolved Hide resolved
pkg/apis/cr/v1alpha1/types.go Outdated Show resolved Hide resolved
pkg/apis/cr/v1alpha1/types.go Outdated Show resolved Hide resolved
pkg/apis/cr/v1alpha1/types.go Outdated Show resolved Hide resolved
pkg/apis/cr/v1alpha1/types.go Outdated Show resolved Hide resolved
pkg/apis/cr/v1alpha1/types.go Outdated Show resolved Hide resolved
Kartik-Garg and others added 17 commits January 6, 2023 16:18
modifying the grammar to make it better

Co-authored-by: Pavan Navarathna <6504783+pavannd1@users.noreply.github.com>
Modified the comments in types.go to make it more understanding.

Co-authored-by: Pavan Navarathna <6504783+pavannd1@users.noreply.github.com>
modifying the comment present for profile

Co-authored-by: Pavan Navarathna <6504783+pavannd1@users.noreply.github.com>
updating comments

Co-authored-by: Pavan Navarathna <6504783+pavannd1@users.noreply.github.com>
Co-authored-by: Pavan Navarathna <6504783+pavannd1@users.noreply.github.com>
Modified the comment present on the Bucket

Co-authored-by: Pavan Navarathna <6504783+pavannd1@users.noreply.github.com>
Modifying the comment present for Prefix

Co-authored-by: Pavan Navarathna <6504783+pavannd1@users.noreply.github.com>
Modifying the comment present on Region

Co-authored-by: Pavan Navarathna <6504783+pavannd1@users.noreply.github.com>
Modifying for Types comment

Co-authored-by: Pavan Navarathna <6504783+pavannd1@users.noreply.github.com>
Modifying comments present on the KeyPair

Co-authored-by: Pavan Navarathna <6504783+pavannd1@users.noreply.github.com>
modified comments present on the username

Co-authored-by: Pavan Navarathna <6504783+pavannd1@users.noreply.github.com>
Modifying comments present on the hostname

Co-authored-by: Pavan Navarathna <6504783+pavannd1@users.noreply.github.com>
modifying the comments present on Key field

Co-authored-by: Pavan Navarathna <6504783+pavannd1@users.noreply.github.com>
Modifying the comment present on the Secret field

Co-authored-by: Pavan Navarathna <6504783+pavannd1@users.noreply.github.com>
Modifying the comments present on Items

Co-authored-by: Pavan Navarathna <6504783+pavannd1@users.noreply.github.com>
modifying comment present on the IDField

Co-authored-by: Pavan Navarathna <6504783+pavannd1@users.noreply.github.com>
Modifying comment present on the field SecretField

Co-authored-by: Pavan Navarathna <6504783+pavannd1@users.noreply.github.com>
@Kartik-Garg
Copy link
Contributor Author

@pavannd1 Thanks for the review comments, I have made the commits according to the suggestion. :D

Copy link
Contributor

@pavannd1 pavannd1 left a comment

Choose a reason for hiding this comment

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

LGTM

@Kartik-Garg
Copy link
Contributor Author

@pavannd1 I just updated the branch with respect to upstream, if everything is in order, could you please merge the PR

@mergify mergify bot merged commit f7975c0 into kanisterio:master Jan 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add comments on the Kanister resource types fields in types.go
4 participants