-
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?
Conversation
Summary of ChangesHello @zhzhuang-zju, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request establishes a new, detailed code style guide for the Karmada project, aiming to enhance code consistency and maintainability across the codebase. By defining explicit rules for Go development, including documentation, interface implementation, function signatures, and secure coding, the guide seeks to streamline contributor onboarding and accelerate the review process. It also introduces standardized formatting for CHANGELOG entries and consolidates existing secure coding guidelines into this central document, providing a single source of truth for coding standards. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
This pull request introduces a new code style guide for the Karmada project, consolidating existing guidelines and adding new ones to ensure consistency. It also includes a Chinese translation. The changes are well-structured and the guide is comprehensive. I've found a couple of minor issues: a sentence in the security section that could be clearer and a recurring typo in an example. My detailed comments provide suggestions for these. Overall, this is a valuable addition to the project's documentation.
i18n/zh/docusaurus-plugin-content-docs/current/developers/code-style-guide.md
Outdated
Show resolved
Hide resolved
i18n/zh/docusaurus-plugin-content-docs/current/developers/code-style-guide.md
Outdated
Show resolved
Hide resolved
97f97fb to
899c4f6
Compare
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
899c4f6 to
2239a48
Compare
Signed-off-by: zhzhuang-zju <[email protected]>
2239a48 to
59d27a5
Compare
RainbowMango
left a comment
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.
/assign
/gemini review
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.
Pull Request Overview
This PR introduces a comprehensive code style guide for the Karmada project, replacing the previous "secure-coding-specifications.md" with an expanded "code-style-guide.md" that includes coding conventions, documentation standards, and release notes formatting guidelines.
- Adds detailed code style guidelines covering documentation, interface compliance, function definitions, and CHANGELOG formatting
- Incorporates the previous security coding specifications as a subsection within the broader style guide
- Provides bilingual documentation (English and Chinese) with practical examples demonstrating correct and incorrect patterns
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
sidebars.js |
Updates sidebar navigation reference from "secure-coding-specifications" to "code-style-guide" |
docs/developers/secure-coding-specifications.md |
Removes the old secure coding specifications document |
docs/developers/code-style-guide.md |
Adds new comprehensive code style guide in English |
i18n/zh/docusaurus-plugin-content-docs/current/developers/secure-coding-specifications.md |
Removes the Chinese version of the old document |
i18n/zh/docusaurus-plugin-content-docs/current/developers/code-style-guide.md |
Adds new comprehensive code style guide in Chinese |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - `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`. |
Copilot
AI
Nov 19, 2025
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.
Inconsistent capitalization in the header. The word "support" should be "supports" to maintain subject-verb agreement.
| - 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`. |
|
|
||
| ## Code Documentation and Commenting | ||
|
|
||
| ### All exported functions, methods, structs, and interfaces **must** be documented with clear and concise comments describing their purpose and behavior. |
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.
Titles should not be too long; as you can see in the preview, it doesn't look right.
We may summarize the title and then elaborate on the details in the main text.
Same as below.
| > 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 |
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.
| ## Code Documentation and Commenting | |
| ## Code Comment |
We don't usually say Code Documentation.
|
|
||
| ## 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. |
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.
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.
| func Foo(bar string, baz int) error | ||
| ``` | ||
|
|
||
| - If a function accepts a `context.Context` parameter, it must be the first argument. |
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.
| - If a function accepts a `context.Context` parameter, it must be the first argument. |
Redundant, this is validated by the Go compiler.
| - `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. |
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.
I don't think dependabot should be omitted. Why?
What type of PR is this?
/kind documentation
What this PR does / why we need it:
Karmada is primarily developed in Go and follows the standard Go community coding conventions. Automated style checks are
enforced via golangci-lint.
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.
Which issue(s) this PR fixes:
Parts of karmada-io/karmada#6922
Special notes for your reviewer: