Skip to content

Conversation

@cce
Copy link
Contributor

@cce cce commented Nov 14, 2025

  • get rid of +build directives
  • put Code generated by ... above package declaration
  • put correct function name in TypeMaxSize() comment

Copy link

Copilot AI left a 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 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 // +build directives in favor of //go:build only
  • 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")
Copy link

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
Copy link

@jannotti jannotti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand the build tags things.

@cce
Copy link
Contributor Author

cce commented Nov 14, 2025

+build was deprecated in Go 1.17 and now things complain if you have it in there

cce added a commit to cce/go-algorand that referenced this pull request Nov 14, 2025
@cce cce merged commit 52531e8 into master Nov 14, 2025
2 checks passed
@cce cce deleted the modernize branch November 14, 2025 17:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants