-
Notifications
You must be signed in to change notification settings - Fork 271
Ensure that all comments are complete sentences as per guidelines #4760
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
|
I wonder if we can put something in |
db022fb to
b9f9ee2
Compare
- Created .github/copilot-instructions.md with detailed guidelines - Addresses comment formatting requirements (complete sentences with periods) - Includes coding conventions, naming standards, and project-specific rules - Covers SAL annotations, error handling patterns, and testing guidelines - Provides 10-point checklist for code generation best practices - Helps ensure Copilot generates code that follows project standards - Responds to suggestion from @dthaler in PR microsoft#4760
|
@dthaler Great suggestion! I've implemented exactly what you requested. I created a comprehensive ✅ Addresses the comment formatting issue: Explicitly instructs Copilot that "All comments MUST be complete sentences ending with proper punctuation (periods)" ✅ Includes all developer guidelines: Covers naming conventions, coding standards, SAL annotations, error handling patterns, testing requirements, and more ✅ Provides practical guidance: 10-point checklist for code generation, examples of correct/incorrect patterns, and project-specific architecture guidelines The file is comprehensive and should significantly help ensure that Copilot-generated code follows the eBPF for Windows project standards, including the comment formatting that this PR addresses. You can see the full implementation in the This should help prevent future PRs from having comment formatting issues and ensure overall better code quality from Copilot contributions! 🚀 |
- Created .github/copilot-instructions.md with detailed guidelines - Addresses comment formatting requirements (complete sentences with periods) - Includes coding conventions, naming standards, and project-specific rules - Covers SAL annotations, error handling patterns, and testing guidelines - Provides 10-point checklist for code generation best practices - Helps ensure Copilot generates code that follows project standards - Responds to suggestion from @dthaler in PR microsoft#4760
f28aee6 to
5d65f3c
Compare
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 pull request standardizes comment formatting across the codebase by ensuring all comments are complete sentences ending with periods. The PR also introduces GitHub Copilot instructions documenting the project's comment guidelines.
Key changes:
- Added periods to incomplete comment sentences throughout multiple files
- Corrected the spelling error "seperately" to "separately" in
bpf_code_generator.cpp - Introduced
.github/copilot-instructions.mdwith project coding guidelines
Reviewed changes
Copilot reviewed 64 out of 64 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
.github/copilot-instructions.md |
New file documenting coding conventions and comment guidelines for GitHub Copilot |
libs/execution_context/ebpf_core.c |
Added period to comment about extern variable |
tools/bpf2c/bpf_code_generator.cpp |
Added periods to comments and corrected spelling error |
tools/bpf2c/bpf2c.cpp |
Added period to comment about hash helper IDs |
tools/bpf2c/bpf2c_dll.c |
Added period to comment about WIN32_LEAN_AND_MEAN |
tools/netsh/dllmain.c |
Added periods to comments about verbs, nouns, and struct field descriptions |
tools/netsh/resource.h |
Added period to comment about next default values |
tests/unit/wer_report_test_wrapper.cpp |
Added periods to test case comments |
tests/stress/km/stress_tests_km.cpp |
Added periods to comments and modified struct initializer spacing |
tests/sample/unsafe/invalid_maps2.c |
Added periods to comments about callee indexes |
tests/libs/util/socket_helper.cpp |
Added periods to multiple comments about socket operations |
tests/bpf2c_tests/expected/*.c (44 files) |
Added periods to WIN32_LEAN_AND_MEAN comments in generated test files |
undocked/tests/sample/ext/inc/sample_ext_ioctls.h |
Added period to comment about IOCTL Codes |
undocked/tests/sample/ext/drv/sample_ext_drv.c |
Added periods to comments and consolidated multi-line comment |
undocked/tests/sample/ext/drv/sample_ext.c |
Added periods to comments about NPI provider characteristics |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Alan Jowett <alan.jowett@microsoft.com>
Signed-off-by: Alan Jowett <alan.jowett@microsoft.com>
Signed-off-by: Alan Jowett <alan.jowett@microsoft.com>
- Created .github/copilot-instructions.md with detailed guidelines - Addresses comment formatting requirements (complete sentences with periods) - Includes coding conventions, naming standards, and project-specific rules - Covers SAL annotations, error handling patterns, and testing guidelines - Provides 10-point checklist for code generation best practices - Helps ensure Copilot generates code that follows project standards - Responds to suggestion from @dthaler in PR microsoft#4760
Remove duplicate naming convention documentation and reference the canonical source in docs/DevelopmentGuide.md instead. This eliminates maintenance fragility and ensures consistency between the two files. Addresses feedback from PR review.
3826b06 to
84f36ed
Compare
…ax in .github/copilot-instructions.md; revert unrelated initializer spacing in tests/stress/km/stress_tests_km.cpp.
|
Addressed review comments:
Pushed to |
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
Copilot reviewed 64 out of 64 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| ## What NOT to Do | ||
|
|
||
| - **DON'T** use global variables where possible. |
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.
Duplicate of what is in https://github.com/microsoft/ebpf-for-windows/blob/main/docs/DevelopmentGuide.md
Can we just point there?
| - Use `const` and `static` modifiers to scope exposure appropriately. | ||
|
|
||
| ### Code Structure | ||
| - Source lines MUST NOT exceed 120 columns. |
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.
Duplicate of what is in https://github.com/microsoft/ebpf-for-windows/blob/main/docs/DevelopmentGuide.md#formatting-notes
Can we just point there?
Description
This pull request primarily updates code comments throughout several files to improve clarity and consistency. Most changes involve adding missing periods at the end of comments or correcting minor typographical errors. There are no functional code changes or logic modifications included.
Comment improvements and typo corrections:
ebpf_core.c,socket_helper.cpp,sample_ext.c,sample_ext_drv.c,sample_ext_ioctls.h, andresource.hto standardize formatting. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11]bpf_code_generator.cppby changing "seperately" to "separately."Testing
CI/CD
Documentation
No.
Installation
No.