🌱 Add an api convention doc #385
Conversation
Signed-off-by: Jian Qiu <[email protected]>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: qiujian16 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
WalkthroughA new documentation file, Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
docs/api-conventions.md (5)
3-4: Fix typos and refine wording in introduction header.Minor editorial nits:
• “guidlines” → “guidelines”
• “inspired from” → “inspired by”-OCM APIs follow the [Kubernetes API conventions](https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md) -with some additional guidlines inspired from [OpenShift API Convention](https://github.com/openshift/enhancements/blob/master/dev-guide/api-conventions.md). +OCM APIs follow the [Kubernetes API conventions](https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md) +with additional guidelines inspired by the [OpenShift API Convention](https://github.com/openshift/enhancements/blob/master/dev-guide/api-conventions.md).
49-50: Use “incompatible” for conciseness.The phrase “not compatible with” can be shortened.
-The JSON tag `omitempty` is not compatible with struct references. +The JSON tag `omitempty` is incompatible with struct references.
75-77: Tighten language around annotation-based APIs.“Seem as” → “seem like”; “usually never happens” is a double negative—prefer “rarely happens”.
-Do not use annotations for extending an API. Annotations may seem as a good candidate for introducing experimental/new -API. Nevertheless, migration from annotations to formal schema usually never happens as it requires breaking changes -in customer deployments. +Do not use annotations to extend an API. Annotations may seem like a good vehicle for introducing experimental or new +functionality, but migration from annotations to a formal schema rarely occurs because it requires breaking changes +in customer deployments.
82-82: Replace “can not” with “cannot”.-4. Hard to extend. An annotation value (a string) can not be extended with additional fields under a parent. +4. Hard to extend. An annotation value (a string) cannot be extended with additional fields under a parent.
88-90: Capitalize “GoDoc” and clarify guidance.Small stylistic tweak and clearer phrasing.
-Ensure that the godoc for a field name matches the JSON name, not the Go name, -in Go definitions for API objects. In particular, this means that the godoc for -field names should use an initial lower-case letter. +Ensure that the GoDoc comment for a field matches its JSON name, not the Go struct field name. +Specifically, the comment should start with the JSON name (initial lower-case letter).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/api-conventions.md(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
docs/api-conventions.md (1)
Learnt from: qiujian16
PR: #380
File: client/work/clientset/versioned/fake/clientset_generated.go:40-40
Timestamp: 2025-07-02T14:09:55.079Z
Learning: Generated code files marked with "Code generated by client-gen. DO NOT EDIT." should not be manually edited as they are automatically generated by Kubernetes code-generator tools and any changes would be overwritten.
🪛 LanguageTool
docs/api-conventions.md
[style] ~49-~49: Consider using “incompatible” to avoid wordiness.
Context: ... be unset. The JSON tag omitempty is not compatible with struct references. Meaning any str...
(NOT_ABLE_PREMIUM)
[style] ~75-~75: Consider using a different adjective in this context to strengthen your wording.
Context: ...nding an API. Annotations may seem as a good candidate for introducing experimental/...
(GOOD_ALTERNATIVE)
[style] ~79-~79: To elevate your writing, try using a synonym here.
Context: ...n. User set values can be too broad and hard to limit later on. 2. Lack of discovera...
(HARD_TO)
[style] ~82-~82: Unless you want to emphasize “not”, use “cannot” which is more common.
Context: ... extend. An annotation value (a string) can not be extended with additional fields unde...
(CAN_NOT_PREMIUM)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: verify
|
Thanks for adding this, very very informative and helpful! |
|
/lgtm Looks good to me! we could have some examples in this doc, but it can be done in another follow-up PR. |
|
/unhold |
openshift-merge-bot
bot
merged commit e861393
into
open-cluster-management-io:main
Summary
Related issue(s)
Fixes #
Summary by CodeRabbit