-
Notifications
You must be signed in to change notification settings - Fork 246
chore: add custom copilot instructions COMPASS-10013 #7628
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
paula-stacho
wants to merge
9
commits into
main
Choose a base branch
from
COMPASS-10013
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.
Open
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
f76e213
chore: add custom copilot instructions COMPASS-10013
paula-stacho 68f87e8
temp: antipattern
paula-stacho 18fc753
logs
paula-stacho 0bea5e6
update instructions
paula-stacho 2f9aded
.
paula-stacho 305a512
freestorage
paula-stacho 99314b7
update instructions
paula-stacho 482f6df
cleanup
paula-stacho 64aa340
more instructions
paula-stacho 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,59 @@ | ||
| For overview, do not summarize the changes. Only provide high level feedback on code quality, performance, and best practices. | ||
|
|
||
| When reviewing code, focus on: | ||
|
|
||
| ## Performance Red Flags | ||
|
|
||
| - Spot inefficient loops and algorithmic issues | ||
| - Check for memory leaks and resource cleanup | ||
|
|
||
| ## Code Quality Essentials | ||
|
|
||
| - Functions should be focused and appropriately sized | ||
| - Use clear, descriptive naming conventions | ||
| - Ensure proper error handling throughout | ||
| - Suggest changes to improve code readability and maintainability | ||
|
|
||
| ## Best practices | ||
|
|
||
| - Refer to official documentation and best practices for React.js, Redux and Node.js. If you see anti-patterns, highlight them and provide links to the relevant official documentation. | ||
|
|
||
| ### Testing | ||
|
|
||
| - Follow the official testing guidelines for Redux and React Testing Library. | ||
| - Ensure tests are meaningful, maintainable, and cover edge cases. | ||
| - Avoid false positive tests. | ||
|
|
||
| ### React | ||
|
|
||
| - Follow React patterns and naming conventions when designing components, “think in React”. | ||
| Be careful when using hooks, make sure that all dependencies are listed for callbacks / memos / effect. | ||
| - Keep in mind that defining non-primitive values in render (in function body or as component props) nullifies all memoization benefits. | ||
| - Same applies to non-primitive properties constructed inside Redux connect functions, they remove any benefits of granular connections | ||
| - Be especially careful when adding effects, remember that “you might not need an effect”. | ||
| - If you’re reaching for a useReducer hook, consider if it’s time to move this state and business logic to a Redux store instead. | ||
|
|
||
| ### Redux | ||
|
|
||
| - The store state should be normalized, minimal, and based around the data you’re storing, not components. | ||
| - Keep in mind that not all state belongs in the redux store | ||
| - Be especially vigilant deciding whether something is state or a derived value produced from multiple existing sources | ||
| - As much state as possible should be calculated in reducers, reducers should own the state shape | ||
| - Store actions should be modeled around events, should not be dispatched in batches, and should handle all complex feature logic, especially the one requiring state and service access, instead of doing this in UI directly | ||
|
|
||
| ## Review Style | ||
|
|
||
| - Be specific and actionable in feedback | ||
| - Explain the "why" behind recommendations | ||
|
|
||
| ## MongoDB Specifics | ||
|
|
||
| ### Performance antipatterns | ||
|
|
||
| - Warn about db.stats() performance implications, especially in connection with freeStorage: 1. Provide a reference to the official MongoDB documentation. | ||
paula-stacho marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| ## Compass Specifics | ||
|
|
||
| - We're trying to stick to Redux style guide as close as possible, with two exceptions: | ||
| - Compass doesn’t use a single Redux store to manage the whole application state. | ||
| - Components inside the feature module boundaries don’t use Redux hooks API as a way to access data from stores (we do allow exposing hooks as public interfaces outside of module boundaries), using connect function instead. | ||
Oops, something went wrong.
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.
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 guess I don't want to give it too much of an opinion but either here or above I would add a mention of using modern JS practices since its been trained on a lot of legacy stuff, like I see copilot not using for-of when it should etc.