Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR modernizes the code generation output to align with current Go conventions. The changes eliminate the deprecated +build directive syntax, relocate the generated code marker to appear before the package declaration (as recommended by Go standards), and fix a documentation comment to include the correct function name.
Key Changes:
- Removed
// +builddirectives in favor of//go:buildonly - Moved "Code generated by..." comment above package declaration
- Added proper function names to MaxSize() method comments
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| printer/print_test.go | Updated test expectation to match new build header format without +build directive |
| printer/print.go | Reordered package header generation to place generated code marker first and removed +build directive |
| gen/maxsize.go | Added function name extraction for MaxSize method comments |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| if IsDangling(p) { | ||
| baseType := p.(*BaseElem).IdentName | ||
| s.p.comment(strings.TrimSuffix(getMaxSizeMethod(baseType), "()") + " returns a maximum valid message size for this message type") |
There was a problem hiding this comment.
The expression strings.TrimSuffix(getMaxSizeMethod(baseType), \"()\") is duplicated on lines 97 and 111. Consider extracting this into a helper function to improve maintainability and reduce code duplication.
jannotti
left a comment
jannotti
left a comment
There was a problem hiding this comment.
I don't understand the build tags things.
|
+build was deprecated in Go 1.17 and now things complain if you have it in there |
+builddirectivesCode generated by ...above package declaration