-
Notifications
You must be signed in to change notification settings - Fork 0
Add Copilot instructions for repository conventions and patterns #8
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
Merged
Merged
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
fd77582
Initial plan
Copilot 663e0bd
Add Copilot instructions for RingBufferCpp repository
Copilot 6205737
Address code review feedback on Copilot instructions
Copilot 2039c23
Clarify Simulate_Android_ToolChain details in Copilot instructions
Copilot 839ed6c
Improve test naming guidance and SFINAE pattern examples
Copilot 5d93a0c
Clarify test naming convention to match project patterns
Copilot 5450282
Update .github/copilot-instructions.md
bugparty 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,109 @@ | ||
| # Copilot Instructions for RingBufferCpp | ||
|
|
||
| ## Project Overview | ||
|
|
||
| This is a modern, header-only C++17 ring buffer (circular buffer) implementation designed for performance, safety, and ease of use. The project demonstrates C++17 best practices and is tested with Google Test. | ||
|
|
||
| ## Architecture | ||
|
|
||
| - **Header-only library**: All implementation is in `RingBuffer.hpp` | ||
| - **Template-based**: Generic implementation using `template<typename T, size_t N, bool Overwrite>` | ||
| - **Namespace**: All code is in the `buffers` namespace | ||
| - **Iterator support**: Custom forward iterators in `buffers::detail` namespace | ||
|
|
||
| ## Code Standards | ||
|
|
||
| ### C++ Version and Style | ||
|
|
||
| - Use **C++17** features and idioms | ||
| - Follow modern C++ best practices: | ||
| - Perfect forwarding with `std::forward` | ||
| - `constexpr if` for compile-time branching | ||
| - Conditional `noexcept` specifications | ||
| - RAII and strong exception safety guarantees | ||
| - `[[nodiscard]]` attributes where appropriate | ||
|
|
||
| ### Naming Conventions | ||
|
|
||
| - **Classes/Types**: `snake_case` (e.g., `ring_buffer`, `ring_buffer_iterator`) | ||
| - **Member variables**: `snake_case_` with trailing underscore (e.g., `source_`, `index_`, `count_`) | ||
| - **Functions/Methods**: `snake_case` (e.g., `push_back`, `pop_front`, `cbegin`) | ||
| - **Template parameters**: PascalCase (e.g., `T`, `N`, `Overwrite`, `C`) | ||
|
|
||
| ### Code Organization | ||
|
|
||
| - Place implementation details in `buffers::detail` namespace | ||
| - Use forward declarations before implementation | ||
| - Include necessary headers at the top (`<iostream>`, `<type_traits>`, `<algorithm>`, `<cstring>`, `<vector>`) | ||
| - Use `#pragma once` for header guards | ||
|
|
||
| ## Building and Testing | ||
|
|
||
| ### Build Commands | ||
|
|
||
| ```bash | ||
| mkdir build && cd build | ||
| cmake .. | ||
| cmake --build . | ||
| ``` | ||
|
|
||
| ### Test Commands | ||
|
|
||
| From within the `build` directory: | ||
|
|
||
| ```bash | ||
| ctest --output-on-failure | ||
| ``` | ||
|
|
||
| ### Test Framework | ||
|
|
||
| - Use **Google Test** framework | ||
| - Test file: `test_main.cpp` | ||
| - GoogleTest is fetched automatically via CMake's `FetchContent` | ||
| - Tests use the pattern: `TEST(RingBufferTest, TestName)` | ||
|
|
||
| ## Testing Guidelines | ||
|
|
||
| When adding or modifying functionality: | ||
|
|
||
| 1. **Write tests first** or alongside implementation | ||
| 2. **Test naming**: The project uses numerical test names (`Test1`, `Test2`) with some descriptive ones (`Test6IteratorOrder`). Follow this convention for consistency. | ||
| 3. **Use Google Test macros**: `EXPECT_EQ`, `EXPECT_TRUE`, `EXPECT_FALSE`, etc. | ||
| 4. **Test edge cases**: | ||
| - Empty buffer | ||
| - Full buffer | ||
| - Overwrite behavior | ||
| - Iterator operations | ||
| - Copy/move semantics | ||
| 5. **Verify both const and non-const operations** where applicable | ||
|
|
||
| ## Key Features to Maintain | ||
|
|
||
| - **Overwrite mode**: When buffer is full, new elements can overwrite oldest | ||
| - **STL-compatible interface**: `begin()`, `end()`, `cbegin()`, `cend()`, `size()`, `empty()` | ||
| - **Constant time operations**: All core operations should be O(1) | ||
| - **Exception safety**: Maintain strong exception guarantees | ||
| - **Type safety**: Use SFINAE and type traits appropriately | ||
|
|
||
| ## Common Patterns in This Codebase | ||
|
|
||
| - Use SFINAE with `typename std::enable_if<condition, int>::type* = nullptr` pattern (e.g., `typename std::enable_if<(!Z), int>::type* = nullptr` for non-const, `typename std::enable_if<(Z), int>::type* = nullptr` for const) | ||
| - Iterator comparisons based on `count()` not `index()` | ||
| - Modulo arithmetic for circular indexing: `index_ = ++index_ % N` | ||
| - Template specialization for const/non-const iterators | ||
|
|
||
| ## What NOT to Do | ||
|
|
||
| - Don't add dependencies beyond standard library (except GoogleTest for tests) | ||
| - Don't break header-only design | ||
| - Don't remove or weaken exception safety guarantees | ||
| - Don't change API to be incompatible with STL conventions | ||
| - Don't add features that would compromise O(1) operation complexity | ||
| - Avoid dynamic allocation; maintain fixed-size design | ||
|
|
||
| ## Special Considerations | ||
|
|
||
| - The buffer size `N` is a compile-time constant (template parameter) | ||
| - Support both trivial and non-trivial types for `T` | ||
| - Handle move-only types appropriately | ||
| - Maintain compatibility with C++17 (the project has a `Simulate_Android_ToolChain` CMake option that when enabled uses C++14 with `-fno-exceptions`, but the default is C++17) | ||
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.
The actual codebase uses both traditional header guards (
#ifndef RINGBUFFERTEST_RINGBUFFER_HPP/#define/#endif) AND#pragma onceon line 11. The instruction here only mentions#pragma once, which could mislead Copilot into not using the traditional header guard pattern that's actually present in the code. Consider updating to reflect that both are used, or remove one from the actual code for consistency.