-
Notifications
You must be signed in to change notification settings - Fork 290
Add copilot-instructions.md #3936
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: develop
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,69 @@ | ||||||||
| ## Code Review Guidelines | ||||||||
|
|
||||||||
| Act as a Senior Staff Engineer reviewing code for the Neural Network Compression Framework (NNCF). | ||||||||
| Be concise, critical, and provide code examples for fixes. | ||||||||
|
|
||||||||
| ## Python Code Style Guidelines | ||||||||
|
|
||||||||
| ### Language Rules | ||||||||
|
|
||||||||
| - Use nested functions/classes only when they make code more readable and simpler, or when closing over local variables. | ||||||||
| - Code must be annotated with type hints (PEP-484) | ||||||||
| - Use `pathlib.Path` instead of `os.*` methods | ||||||||
| - Avoid using `assert` statements in `src/nncf`, raise proper exceptions with informative messages instead | ||||||||
|
|
||||||||
| ### Style Rules | ||||||||
|
|
||||||||
| **Format of docstrings**: | ||||||||
|
|
||||||||
| ```python | ||||||||
| def foo(arg1: int, arg2: bool = False) -> int: | ||||||||
| """ | ||||||||
| Brief description of what the function does. | ||||||||
|
|
||||||||
| :param arg1: Description | ||||||||
| :param arg2: Description | ||||||||
| :return: Description | ||||||||
| """ | ||||||||
| ``` | ||||||||
|
|
||||||||
| - All externally visible functions need docstrings unless very short, obvious, and not externally visible | ||||||||
|
|
||||||||
| **Naming convention** | ||||||||
|
|
||||||||
| - Modules/Packages: `lower_with_under` | ||||||||
| - Classes: `CapWords` | ||||||||
| - Functions/Methods: `lower_with_under()` | ||||||||
| - Constants: `CAPS_WITH_UNDER` | ||||||||
| - Variables: `lower_with_under` | ||||||||
| - Internal/Private: prefix with `_` | ||||||||
| - Avoid single char names (except `i j k v` for loops, `e` for exceptions, `f` for files, `a b x y z` for tensors) | ||||||||
| - Avoid dashes in package/module names | ||||||||
| - Avoid type suffixes in names (e.g., `id_to_name_dict`) | ||||||||
|
|
||||||||
| **File naming** | ||||||||
|
|
||||||||
| - Use `.py` extension, never dashes | ||||||||
| - Avoid generic names like `utils.py` or `helpers.py` | ||||||||
| - Group related code in dedicated, explicitly-named modules | ||||||||
|
|
||||||||
| ### Test Suite Rules | ||||||||
|
|
||||||||
| **Basic Style** | ||||||||
|
|
||||||||
| - Use pytest framework | ||||||||
| - Type-hint all parameters and fixtures | ||||||||
| - Use fixtures, not xunit-style setup/teardown | ||||||||
|
|
||||||||
| ### Review Focus Areas | ||||||||
|
|
||||||||
| When reviewing code or suggesting changes, adhere to the following logic-first rules: | ||||||||
|
|
||||||||
| - Look for performance issues, potential bottlenecks and memory leaks | ||||||||
|
Contributor
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
😆
Collaborator
Author
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. Since artificial intelligence is integrated into the IDE, it can be assumed that each PR includes machine-generated code.
Collaborator
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 manual review is also required by SDL.
Contributor
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. probably phrasing is not right. The idea was to create an instruction to catch/filter PR's that were generated fully by bots (without participation of humans). |
||||||||
| - Follow python code style guidelines | ||||||||
| - Grammar and clarity in docstrings and variable names | ||||||||
| - Proper use of type hints and annotations | ||||||||
| - Avoid comment about formatting issues, focus on code logic and style | ||||||||
AlexanderDokuchaev marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||
| - Ensure that appropriate tests are added for the new code, and that they are well-structured, clear, and comply with the testing guidelines | ||||||||
| - Check for proper exception handling and informative error messages | ||||||||
| - Ensure that code is modular, reusable, and avoids unnecessary complexity | ||||||||
|
Contributor
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
Collaborator
Author
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. Without a definition of a public API, this is too general a concept.
Contributor
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. agree, it's hard to define, but it doesn't mean that it's not needed. what about this phrasing?
|
||||||||
Uh oh!
There was an error while loading. Please reload this page.