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

[ISSUE-5] fix docs; add missing descriptions #6

Merged

Conversation

jameskim0987
Copy link
Contributor

@jameskim0987 jameskim0987 commented Oct 2, 2024

What type of PR is this?
/kind documentation

What does this PR do / why we need it:

  1. Add missing field descriptions in https://doc.crds.dev/github.com/argoproj-labs/[email protected] CRDs

Have you updated the necessary documentation?

  • Documentation update is required by this PR.
  • Documentation has been updated.

Which issue(s) this PR fixes:

Fixes #5

How to test changes / Special notes to the reviewer:

  1. The changes can be observed here: https://doc.crds.dev/github.com/jameskim0987/[email protected]. ( NOTE: after this PR gets merged. The repo release will need to be bumped so that it can be referenced with the latest changes from https://doc.crds.dev ).

@ggkhrmv
Copy link
Collaborator

ggkhrmv commented Oct 2, 2024

@jameskim0987 please add documentation to "api/v1alpha1/argocdrole_types.go" and RoleBinding. You can look at status.go for valid structure of documentation.

@jameskim0987 jameskim0987 force-pushed the issue-5-fix-docs-missing-descriptions branch 2 times, most recently from 76eee20 to 88a8864 Compare October 2, 2024 20:32
@ggkhrmv ggkhrmv self-requested a review October 2, 2024 20:36
@jameskim0987 jameskim0987 marked this pull request as ready for review October 2, 2024 20:54
@jameskim0987
Copy link
Contributor Author

@ggkhrmv This PR is ready for review. Let me know if I missed any or need to modify the comments!

Copy link
Collaborator

@ggkhrmv ggkhrmv left a comment

Choose a reason for hiding this comment

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

@jameskim0987 I left a few comments and suggestions, please incorporate them, after that I can approve the PR :)

items:
type: string
type: array
resource:
description: Target resource type.
Copy link
Collaborator

Choose a reason for hiding this comment

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

@jameskim0987 could you please add those comments (objects, resource, verbs, ...) to the api/v1alpha1/argocdrole_types.go? otherwise the changes will be overriden, if we change something in the types file and generate the yaml

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure thing! Done.

@ggkhrmv ggkhrmv added the documentation Improvements or additions to documentation label Oct 4, 2024
@jameskim0987 jameskim0987 force-pushed the issue-5-fix-docs-missing-descriptions branch from 0535c99 to e1ec49c Compare October 4, 2024 15:53
Copy link
Collaborator

@ggkhrmv ggkhrmv left a comment

Choose a reason for hiding this comment

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

@jameskim0987 please make the changes and run make manifests and make install. If there are any file changes, please also commit them :)

@jameskim0987 jameskim0987 force-pushed the issue-5-fix-docs-missing-descriptions branch from f3c9511 to 5c117a2 Compare October 4, 2024 17:23
@jameskim0987
Copy link
Contributor Author

jameskim0987 commented Oct 4, 2024

@jameskim0987 please make the changes and run make manifests and make install. If there are any file changes, please also commit them :)

Thank you for the pointers!

While running make manifests and make install, I noticed that some fields were getting misplaced so I manually moved them back to the correct locations on this commit here: 5c117a2

I completed making the changes and the changes can be viewed at https://doc.crds.dev/github.com/jameskim0987/[email protected]

Copy link
Collaborator

@ggkhrmv ggkhrmv left a comment

Choose a reason for hiding this comment

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

LGTM

@ggkhrmv
Copy link
Collaborator

ggkhrmv commented Oct 4, 2024

@jameskim0987 please make the changes and run make manifests and make install. If there are any file changes, please also commit them :)

Thank you for the pointers!

While running make manifests and make install, I noticed that some fields were getting misplaced so I manually moved them back to the correct locations on this commit here: 5c117a2

I completed making the changes and the changes can be viewed at https://doc.crds.dev/github.com/jameskim0987/[email protected]

@jameskim0987 LGTM, thanks for your help :)

@ggkhrmv ggkhrmv merged commit 72cc714 into argoproj-labs:main Oct 4, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[docs] add description for CRD fields
2 participants