-
Notifications
You must be signed in to change notification settings - Fork 149
add karmada code style guide #922
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,171 @@ | ||||||
| --- | ||||||
| title: Karmada Code Style Guide | ||||||
| --- | ||||||
|
|
||||||
| Karmada is primarily developed in Go and follows the standard Go community coding conventions. Automated style checks are | ||||||
| enforced via [golangci-lint](https://github.com/karmada-io/karmada/blob/e2c4b596a5da442fc0dbeab9f9063d8db8669208/.github/workflows/ci.yml#L18-L35). | ||||||
| However, the resulting code can still look and feel very differently among different developers. To ensure consistency | ||||||
| across the codebase, this guide defines additional rules beyond linter defaults. | ||||||
|
|
||||||
| Adhering to this guide helps new contributors onboard quickly, reduces PR review iterations, shortens maintainer review | ||||||
| cycles, and ultimately speeds up merge velocity. | ||||||
|
|
||||||
| > Note: This guide is not a replacement for the official Go style guide but rather a supplement to it. All aspects not | ||||||
| > covered in this guide should follow the official Go style guide and best practices. | ||||||
| > Additionally, existing code in the Karmada codebase may not fully comply with this guide. We encourage adherence to | ||||||
| > this guide when writing new code or modifying existing code to gradually improve code quality and consistency. | ||||||
|
|
||||||
| ## Code Documentation and Commenting | ||||||
|
|
||||||
| ### All exported functions, methods, structs, and interfaces **must** be documented with clear and concise comments describing their purpose and behavior. | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Titles should not be too long; as you can see in the preview, it doesn't look right. Same as below. |
||||||
|
|
||||||
| WRONG | ||||||
| ```go | ||||||
| func GetAnnotationValue(annotations map[string]string, annotationKey string) string { | ||||||
| if annotations == nil { | ||||||
| return "" | ||||||
| } | ||||||
| return annotations[annotationKey] | ||||||
| } | ||||||
| ``` | ||||||
|
|
||||||
| RIGHT | ||||||
| ```go | ||||||
| // GetAnnotationValue retrieves the value via 'annotationKey' (if it exists), otherwise an empty string is returned. | ||||||
| func GetAnnotationValue(annotations map[string]string, annotationKey string) string { | ||||||
| if annotations == nil { | ||||||
| return "" | ||||||
| } | ||||||
| return annotations[annotationKey] | ||||||
| } | ||||||
| ``` | ||||||
|
|
||||||
| ### Comments in the body of the code are highly encouraged, but they should explain the intention of the code as opposed to just calling out the obvious. | ||||||
|
|
||||||
| WRONG | ||||||
| ```go | ||||||
| // continue if the cluster is deleting | ||||||
| if !c.Cluster().DeletionTimestamp.IsZero() { | ||||||
| klog.V(4).Infof("Cluster %q is deleting, skip it", c.Cluster().Name) | ||||||
| continue | ||||||
| ``` | ||||||
|
|
||||||
| RIGHT | ||||||
| ```go | ||||||
| // When cluster is deleting, we will clean up the scheduled results in the cluster. | ||||||
| // So we should not schedule resource to the deleting cluster. | ||||||
| if !c.Cluster().DeletionTimestamp.IsZero() { | ||||||
| klog.V(4).Infof("Cluster %q is deleting, skip it", c.Cluster().Name) | ||||||
| continue | ||||||
| ``` | ||||||
|
|
||||||
| ## Interface Compliance | ||||||
|
|
||||||
| ### Any struct that explicitly implements an interface must include a compile-time interface compliance check. | ||||||
|
|
||||||
| using the following pattern: | ||||||
| ```go | ||||||
| var _ InterfaceName = &StructName{} | ||||||
| ``` | ||||||
| This assertion should be placed in the same file as the struct definition (typically near the type declaration or at the top of the file) to ensure: | ||||||
| 1. Compilation fails immediately if the struct does not fully implement the interface; | ||||||
| 2. The interface contract is explicitly declared, improving readability and maintainability. | ||||||
|
|
||||||
| RIGHT | ||||||
| ```go | ||||||
| // Check if our workloadInterpreter implements necessary interface | ||||||
| var _ interpreter.Handler = &workloadInterpreter{} | ||||||
|
|
||||||
| // workloadInterpreter explore resource with request operation. | ||||||
| type workloadInterpreter struct { | ||||||
| decoder *interpreter.Decoder | ||||||
| } | ||||||
| ``` | ||||||
|
|
||||||
| ## Function Definitions | ||||||
| ### When a function has many parameters, consider encapsulating them into a struct to improve readability and maintainability. | ||||||
| ### Function signatures should preferably be written on a single line, including the parameter list and return types. | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The underlying reasons should be explained: having too many parameters in a function can harm its readability. Generally, a function should not have more than 5 parameters. If it exceeds this, consider refactoring the function or encapsulating the parameters. |
||||||
|
|
||||||
| WRONG | ||||||
| ```go | ||||||
| func Foo( | ||||||
| bar string, | ||||||
| baz int) error | ||||||
| ``` | ||||||
|
|
||||||
| RIGHT | ||||||
| ```go | ||||||
| func Foo(bar string, baz int) error | ||||||
| ``` | ||||||
|
|
||||||
| - If a function accepts a `context.Context` parameter, it must be the first argument. | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Redundant, this is validated by the Go compiler. |
||||||
|
|
||||||
| ## Secure Coding Specifications | ||||||
|
|
||||||
| ### It is prohibited to have authentication credentials that cannot be modified (e.g., hard-coded passwords in process binaries). | ||||||
| ### If implemented using interpreted languages (such as Shell/Python/Perl scripts, JSP, HTML, etc.), for functions that need to be cleaned up, they must be completely deleted. It is strictly prohibited to use forms such as comment lines to merely disable the functions. | ||||||
| ### It is prohibited to use private cryptographic algorithms for encryption and decryption, including: | ||||||
|
|
||||||
| - Cryptographic algorithms designed independently without being evaluated by professional institutions; | ||||||
| - Self-defined data conversion algorithms executed through methods such as deformation/character shifting/replacement; | ||||||
| - Pseudo-encryption implementations that use encoding methods (such as Base64 encoding) to achieve the purpose of data encryption. Note: In scenarios other than encryption and decryption, the use of encoding methods such as Base64 or algorithms such as deformation/shifting/replacement for legitimate business purposes does not violate this provision. | ||||||
|
|
||||||
| ### The random numbers used in cryptographic algorithms must be secure random numbers in the cryptographic sense. | ||||||
| ### It is prohibited to print authentication credentials (passwords/private keys/pre-shared keys) in plain text in system-stored logs, debugging information, and error prompts. | ||||||
|
|
||||||
| ## CHANGELOG(Release Notes) Style Guide | ||||||
|
|
||||||
| ### Path | ||||||
| The `CHANGELOG` files are located under `docs/CHANGELOG/` in the Karmada repository. | ||||||
|
|
||||||
| ### Responsibilities | ||||||
| The `CHANGELOG` files are used to record release notes for each version, including new features, bug fixes, deprecations, and other significant changes. | ||||||
|
|
||||||
| ### Code Style | ||||||
| #### Release notes for the same version should be grouped by category (e.g., Features, Bug Fixes, Deprecations). | ||||||
| #### Each release note follows one of two formats, depending on whether it relates to a specific component. | ||||||
|
|
||||||
| ```markdown | ||||||
| - `Component`: Description ([#pr_ID](pr_link), @author) | ||||||
| - Description ([#pr_ID](pr_link), @author) | ||||||
| ``` | ||||||
| > Note: If the author is `dependabot`, the `, @author` part should be omitted. | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think dependabot should be omitted. Why? |
||||||
|
|
||||||
| RIGHT | ||||||
| ```markdown | ||||||
| - `karmada-controller-manager`: Fixed the issue that a workload propagated with `duplicated mode` can bypass quota checks during scale up. ([#6474](https://github.com/karmada-io/karmada/pull/6474), @zhzhuang-zju) | ||||||
| ``` | ||||||
|
|
||||||
| #### Component names must be wrapped in backticks (e.g., `karmada-controller-manager`). | ||||||
| #### Within each category, release notes should be grouped by component to improve readability. | ||||||
|
|
||||||
| WRONG | ||||||
| ```markdown | ||||||
| - `karmada-controller-manager`: Fixed the issue that reporting repeat EndpointSlice resources leads to duplicate backend IPs. ([#6693](https://github.com/karmada-io/karmada/pull/6693), @XiShanYongYe-Chang) | ||||||
| - `karmada-interpreter-webhook-example`: Fixed write response error for broken HTTP connection issue. ([#6680](https://github.com/karmada-io/karmada/pull/6680), @tdn21) | ||||||
| - `karmada-controller-manager`: Fixed the issue that the relevant fields in rb and pp are inconsistent. ([#6714](https://github.com/karmada-io/karmada/pull/6714), @zhzhuang-zju) | ||||||
| ``` | ||||||
|
|
||||||
| RIGHT | ||||||
| ```markdown | ||||||
| - `karmada-controller-manager`: Fixed the issue that reporting repeat EndpointSlice resources leads to duplicate backend IPs. ([#6693](https://github.com/karmada-io/karmada/pull/6693), @XiShanYongYe-Chang) | ||||||
| - `karmada-controller-manager`: Fixed the issue that the relevant fields in rb and pp are inconsistent. ([#6714](https://github.com/karmada-io/karmada/pull/6714), @zhzhuang-zju) | ||||||
| - `karmada-interpreter-webhook-example`: Fixed write response error for broken HTTP connection issue. ([#6680](https://github.com/karmada-io/karmada/pull/6680), @tdn21) | ||||||
| ``` | ||||||
|
|
||||||
| #### Tense usage | ||||||
|
|
||||||
| - `Deprecations`: Use present perfect tense (e.g., “has been deprecated”). | ||||||
| - `Dependencies`: Use present perfect tense or past tense (e.g., “has been upgraded to…” or “Upgraded to…”). | ||||||
| - All other categories (features, fixes, etc.): Use simple past tense (e.g., “Fixed…”, “Added…”, “Removed…”). | ||||||
| - Only when describing a newly introduced capability or behavioral changes, you may use present tense constructions like `now support` or `no longer relies`. | ||||||
|
||||||
| - Only when describing a newly introduced capability or behavioral changes, you may use present tense constructions like `now support` or `no longer relies`. | |
| - Only when describing a newly introduced capability or behavioral changes, you may use present tense constructions like `now supports` or `no longer relies`. |
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,168 @@ | ||
| --- | ||
| title: Karmada 代码风格指南 | ||
| --- | ||
|
|
||
| Karmada 主要使用 Go 语言开发,遵循 Go 社区通用代码风格,并通过 [golangci-lint](https://github.com/karmada-io/karmada/blob/e2c4b596a5da442fc0dbeab9f9063d8db8669208/.github/workflows/ci.yml#L18-L35) | ||
| 工具进行自动化检查。然而,不同开发者编写的代码的风格可能最终大相径庭。我们旨在补充若干强制性规范,以统一 Karmada 代码的风格,从而帮助提高整体可读性。 | ||
|
|
||
| 遵循本指南有助于新贡献者快速融入项目,减少 PR 往返,并缩短维护者的审核周期,从而加速合入流程。 | ||
|
|
||
| > 注意:本指南并非 Go 语言官方风格指南的替代品,而是对其的补充。所有不在本指南中提及的内容均应遵循 Go 语言官方风格指南和最佳实践。 | ||
| > 此外,Karmada 代码库中现有的代码可能未完全符合本指南。我们鼓励在编写新代码或修改现有代码时遵循本指南,以逐步提升代码质量和一致性。 | ||
|
|
||
| ## 代码注释 | ||
|
|
||
| ### 所有导出的函数、方法、结构体和接口都必须有注释,注释应简洁明了地描述其功能和用途。 | ||
|
|
||
| 错误示例 | ||
| ```go | ||
| func GetAnnotationValue(annotations map[string]string, annotationKey string) string { | ||
| if annotations == nil { | ||
| return "" | ||
| } | ||
| return annotations[annotationKey] | ||
| } | ||
| ``` | ||
|
|
||
| 正确示例 | ||
| ```go | ||
| // GetAnnotationValue retrieves the value via 'annotationKey' (if it exists), otherwise an empty string is returned. | ||
| func GetAnnotationValue(annotations map[string]string, annotationKey string) string { | ||
| if annotations == nil { | ||
| return "" | ||
| } | ||
| return annotations[annotationKey] | ||
| } | ||
| ``` | ||
|
|
||
| ### 鼓励在代码内部添加注释,但应解释设计意图或复杂逻辑,而非解释显而易见的操作。 | ||
|
|
||
| 错误示例 | ||
| ```go | ||
| // continue if the cluster is deleting | ||
| if !c.Cluster().DeletionTimestamp.IsZero() { | ||
| klog.V(4).Infof("Cluster %q is deleting, skip it", c.Cluster().Name) | ||
| continue | ||
| ``` | ||
|
|
||
| 正确示例 | ||
| ```go | ||
| // When cluster is deleting, we will clean up the scheduled results in the cluster. | ||
| // So we should not schedule resource to the deleting cluster. | ||
| if !c.Cluster().DeletionTimestamp.IsZero() { | ||
| klog.V(4).Infof("Cluster %q is deleting, skip it", c.Cluster().Name) | ||
| continue | ||
| ``` | ||
|
|
||
| ## 接口合规性 | ||
|
|
||
| ### 所有显式实现接口的结构体,必须在代码中添加编译时接口合规性检查,以确保完整实现接口契约。 | ||
|
|
||
| 使用以下模式: | ||
| ```go | ||
| var _ InterfaceName = &StructName{} | ||
| ``` | ||
| 此断言应置于结构体定义所在的文件中(通常靠近类型声明或文件顶部),确保: | ||
|
|
||
| 1.若结构体未完整实现接口,编译阶段即报错; | ||
|
|
||
| 2.显式表达该结构体的接口契约,提升代码可读性与可维护性。 | ||
|
|
||
| 正确示例 | ||
| ```go | ||
| // Check if our workloadInterpreter implements necessary interface | ||
| var _ interpreter.Handler = &workloadInterpreter{} | ||
|
|
||
| // workloadInterpreter explore resource with request operation. | ||
| type workloadInterpreter struct { | ||
| decoder *interpreter.Decoder | ||
| } | ||
| ``` | ||
|
|
||
| ## 函数定义 | ||
| ### 函数参数过多时,建议使用结构体封装参数,以提高代码可读性和可维护性。 | ||
| ### 函数签名应优先写在单行,包括参数列表和返回值。 | ||
|
|
||
| 错误示例 | ||
| ```go | ||
| func Foo( | ||
| bar string, | ||
| baz int) error | ||
| ``` | ||
|
|
||
| 正确示例 | ||
| ```go | ||
| func Foo(bar string, baz int) error | ||
| ``` | ||
|
|
||
| ### 当接受 `context.Context` 参数时,必须将其作为函数的第一个参数。 | ||
|
|
||
| ## 安全编码规范 | ||
|
|
||
| ### 禁止存在无法修改的认证凭据(如:进程二进制中硬编码口令)。 | ||
| ### 如果采用解释性语言(如 Shell/Python/Perl 脚本、JSP、HTML等)实现,对于需要清理的功能,必须彻底删除,严禁使用注释行等形式仅使功能失效。 | ||
| ### 禁止使用私有密码算法实现加解密,包括: | ||
|
|
||
| - 未经过专业机构评估的、自行设计的密码算法; | ||
| - 自行定义的通过变形/字符移位/替换等方式执行的数据转换算法; | ||
| - 用编码的方式(如 Base64 编码)实现数据加密的目的的伪加密实现。 说明:在非加解密场景,出于正常业务目的使用 Base64 等编码方式或变形/移位/替换等算法不违反此条。 | ||
|
|
||
| ### 密码算法中使用到的随机数必须是密码学意义上的安全随机数。 | ||
| ### 禁止在系统中存储的日志、调试信息、错误提示中明文打印认证凭据(口令/私钥/预共享密钥)。 | ||
|
|
||
| ## CHANGELOG (release notes) 格式规范 | ||
|
|
||
| ### 文件路径 | ||
| `CHANGELOG` 文件位于 Karmada 仓库的 `docs/CHANGELOG` 目录下。 | ||
|
|
||
| ### 职责 | ||
| 用于记录每个版本的发布说明(release notes),包括新增功能、问题修复和其他重要变更。 | ||
|
|
||
| ### 格式规范 | ||
| #### 同版本的 release notes 按照类别组合。 | ||
| #### 每条 release note 可按是否涉及具体组件而分为两种格式: | ||
|
|
||
| ```markdown | ||
| - `组件名`: 内容 ([#pr编号](pr链接), @pr作者) | ||
| - 内容 ([#pr编号](pr链接), @pr作者) | ||
| ``` | ||
| > 注:如果 pr作者为 `dependabot`,可省略 `, @pr作者` 部分。 | ||
|
|
||
| 正确示例 | ||
| ```markdown | ||
| - `karmada-controller-manager`: Fixed the issue that a workload propagated with `duplicated mode` can bypass quota checks during scale up. ([#6474](https://github.com/karmada-io/karmada/pull/6474), @zhzhuang-zju) | ||
| ``` | ||
|
|
||
| #### 组件名使用反引号标记,例如 `karmada-controller-manager`。 | ||
| #### 同一类别的 release notes 按照组件归类,提高可读性。 | ||
|
|
||
| 错误示例 | ||
| ```markdown | ||
| - `karmada-controller-manager`: Fixed the bug that XXXXX. ([#6446](https://github.com/karmada-io/karmada/pull/6446), @XiShanYongYe-Chang) | ||
| - `helm`: Fixed the issue where `helm upgrade` failed to update Karmada's static resources properly. ([#6395](https://github.com/karmada-io/karmada/pull/6395), @deefreak) | ||
| - `karmada-controller-manager`: Fixed the issue that `taint-manager` didn't honour`--no-execute-taint-eviction-purge-mode` when evicting `ClusterResourceBinding`. ([#6491](https://github.com/karmada-io/karmada/pull/6491), @XiShanYongYe-Chang) | ||
| ``` | ||
|
|
||
| 正确示例 | ||
| ```markdown | ||
| - `karmada-controller-manager`: Fixed the bug that cluster can not be unjoined in case of the `--enable-taint-manager=false` or the feature gate `Failover` is disabled. ([#6446](https://github.com/karmada-io/karmada/pull/6446), @XiShanYongYe-Chang) | ||
| - `karmada-controller-manager`: Fixed the issue that `taint-manager` didn't honour`--no-execute-taint-eviction-purge-mode` when evicting `ClusterResourceBinding`. ([#6491](https://github.com/karmada-io/karmada/pull/6491), @XiShanYongYe-Chang) | ||
| - `helm`: Fixed the issue where `helm upgrade` failed to update Karmada's static resources properly. ([#6395](https://github.com/karmada-io/karmada/pull/6395), @deefreak) | ||
| ``` | ||
|
|
||
| #### 注意时态使用 | ||
|
|
||
| - 废弃(Deprecation)类 release note 使用现在完成时态,表示某个功能已经被废弃。 | ||
| - 依赖(Dependencies)类 release note 可使用现在完成时态或过去时态,表示某个依赖已经被升级。比如 “has been upgraded to…” 或 “Upgraded to…”。 | ||
| - 其它类别统一使用一般过去时,表示某个功能已经被添加或修复。 | ||
| - 仅当描述“从当前版本起引入的新能力或行为变更”时,可使用 `now supports`、`no longer relies` 等一般现在时结构。 | ||
|
|
||
| 错误示例 | ||
| ```markdown | ||
| - `karmada-controller-manager`: Fix the bug that xxx. ([#prID](pr_link), @author) | ||
| ``` | ||
|
|
||
| 正确示例 | ||
| ```markdown | ||
| - `karmada-controller-manager`: Fixed the bug that xxx. ([#prID](pr_link), @author) | ||
| ``` |
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't usually say
Code Documentation.