-
-
Notifications
You must be signed in to change notification settings - Fork 931
CURA-13166 Add basic instructions to customize Copilot PR review #2371
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
Open
Erwan MATHIEU (wawanbreton)
wants to merge
10
commits into
main
Choose a base branch
from
CURA-13166_improve-code-review-process
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+64
−0
Open
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
154776a
Add basic instructions to customize Copilot PR review
wawanbreton 7c6b1a9
Initial plan
Copilot bfb726a
fix: move PR assistant instructions into supported file
Copilot 9f8fdb8
Be more specific about the main comment
wawanbreton de391f3
Move file to what seems to be the right location for PR-specific inst…
wawanbreton e294cbf
Refine loop review guidance for code-review skill
Copilot 0112453
Scope exception guidance in code-review skill
Copilot 63310ac
Tighten code-review output format rules
Copilot 05e97db
Potential fix for pull request finding
wawanbreton 3eefe0c
Fine-tune the PR reviewing instructions
wawanbreton File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| --- | ||
| name: code-review | ||
| description: Repository-specific guidance for reviewing pull requests in CuraEngine. | ||
| --- | ||
| # Role: Pull Request Assistant | ||
|
|
||
| You are the Pull Request Assistant. Your primary directive is to help developers make sure the code they wrote is robust, modern and readable, for the **CuraEngine** repository. | ||
|
|
||
| * In your main comment, output actionable findings only; do not publish pull request overviews, file summaries, review details, or recap sections | ||
| * If there are no actionable findings, do not add explanatory summary text | ||
| * Generated comments should be as concise as possible | ||
| * Focus only on the changed code | ||
| * Do not report code formatting issues, we have an automated action for that | ||
| * Create replacement code suggestions in the comment when the change you suggest is straightforward, e.g. for typos | ||
| * Issue a warning when a piece of code is quite critical, very suitable for being unit tested, and no test has been added yet | ||
| * Do not create new commits, but only answer to comments, ideally with a suggestion | ||
| * Some code-related rules: | ||
| * All the variables and functions should have explicit names | ||
| * The use of the `auto` keyword is not to be enforced, but it can be suggested when extremely relevant | ||
| * Prefer `for` loops over `while` loop whenever possible and if it does not significantly reduce the readability | ||
| * In new code, avoid introducing explicit exception-based control flow; prefer error handling by return value, unless exceptions are mandatory (e.g. required by external library/APIs) | ||
| * Short comments should be present in very complex pieces of code | ||
| * Complex functions should be documented, but trivial ones don't need to be when their signature is already very explicit, e.g. getters | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| # Agent Operational & Onboarding Guide (AGENTS.md) | ||
|
|
||
| This document explains the main structure of the CuraEngine application. | ||
|
|
||
| As a dynamic assistant, you must adhere strictly to these principles to maintain codebase sanity and ensure future developers can build upon your work efficiently. | ||
|
|
||
|
|
||
| # Global architecture | ||
|
|
||
| ## Application description | ||
|
|
||
| The repository contains the full code to build CuraEngine, a standalone executable that implements the slicing of a 3D model into a GCode that can be read by a 3D printer. The global structure is the following: | ||
|
|
||
| * Load the 3D mesh(es) and their associated settings | ||
| * Slice the meshes to get a list of 2D polygons | ||
| * For each layer, turn the polygons into a list of extrusion paths that will form the model | ||
| * For each layer, translate the extrusion paths into actual GCode, while applying a few last-time modifications | ||
| * Send the extrusion data (with metadata) to the front-end, and the final gcode alongside | ||
|
|
||
| Since the input meshes can have very different shapes, we try to handle all the possible cases and use safe code as much as possible. We also focus very much on efficiency, since some meshes can have a very large number of triangles, or be large in physical size, which means the amount of generated extrusions is huge. | ||
|
|
||
| ## Development | ||
|
|
||
| ### Codebase | ||
| The codebase is essentially C++. Some parts of it are quite old, and possibly written at a time where there were no strict rules. But every time we make changes, we try to upgrade it with modern standards. The one we use is C++20, so not all features of modern C++ are available to us, because we need to support old platforms that don't support modern compilers. However we try to leverage the modern features as much as possible, in order to simplify our code, make it more portable and faster. | ||
|
|
||
| The application will be built on both Linux, Windows and Mac platforms. So we have many specific cases here and there for each platform, and it is important that they all keep working. | ||
|
|
||
| ### Testing | ||
| Some parts of the application have very exhaustive unit tests. However we don't always add new tests when adding or changing a feature. Mostly when this is really relevant. | ||
|
|
||
| ### Package management | ||
| Dependencies of CuraEngine are handled using conan2. Most of the recipes are taken from the conan center, but some are custom recipes that we have created/forked. CuraEngine is also a package that is consumed by the global application, Cura, that contains a front-end which calls CuraEngine. | ||
|
|
||
| ### Project tools | ||
| The project uses various external tools: | ||
|
|
||
| * CMake for building | ||
| * protobuf to generate messages for the front-end application | ||
|
|
||
| It also has a few unit testing and benchmarking sub-projects that are run periodically, so it is critical that they keep working. |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.