-
Notifications
You must be signed in to change notification settings - Fork 12
HYPERFLEET-375: add dependency pinning standard with Bingo #72
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
WalkthroughA new standards documentation file is added that specifies the Dependency and Tool Version Pinning approach for HyperFleet repositories using Bingo. The document outlines the problem of inconsistent tool versions affecting build reproducibility, presents Bingo as the solution, and provides comprehensive implementation guidance including directory structure, file naming conventions, Makefile integration patterns, CI integration examples using GitHub Actions, quick start instructions, command references for tool management, and practical examples for common development tools such as linters, code generators, and protocol buffer compilers. Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
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.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @hyperfleet/standards/dependency-pinning.md:
- Around line 238-265: The Makefile examples in the markdown use hard tabs
inside the fenced code blocks (e.g., recipe lines under targets like tools-list,
tools-update, lint, generate and variable references $(GOLANGCI_LINT),
$(MOCKGEN), $(OAPI_CODEGEN)); replace those hard tabs with spaces so the
documentation code blocks use only spaces for indentation (or reformat the
blocks to use triple-backtick fenced blocks with space-indented lines) while
keeping the actual Makefile semantics intact in real files—update every recipe
line and indented line in those examples to use spaces instead of \t.
- Line 34: The fenced code block at the start of the example (the ``` fence)
lacks a language identifier; update that fence to include a language specifier
(use "text") so the block becomes ```text to enable proper syntax highlighting
and improve readability.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
hyperfleet/standards/dependency-pinning.md
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
hyperfleet/standards/dependency-pinning.md
34-34: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
105-105: Hard tabs
Column: 1
(MD010, no-hard-tabs)
144-144: Hard tabs
Column: 1
(MD010, no-hard-tabs)
238-238: Hard tabs
Column: 1
(MD010, no-hard-tabs)
239-239: Hard tabs
Column: 1
(MD010, no-hard-tabs)
240-240: Hard tabs
Column: 1
(MD010, no-hard-tabs)
241-241: Hard tabs
Column: 1
(MD010, no-hard-tabs)
242-242: Hard tabs
Column: 1
(MD010, no-hard-tabs)
247-247: Hard tabs
Column: 1
(MD010, no-hard-tabs)
252-252: Hard tabs
Column: 1
(MD010, no-hard-tabs)
253-253: Hard tabs
Column: 1
(MD010, no-hard-tabs)
254-254: Hard tabs
Column: 1
(MD010, no-hard-tabs)
255-255: Hard tabs
Column: 1
(MD010, no-hard-tabs)
256-256: Hard tabs
Column: 1
(MD010, no-hard-tabs)
261-261: Hard tabs
Column: 1
(MD010, no-hard-tabs)
265-265: Hard tabs
Column: 1
(MD010, no-hard-tabs)
286-286: Hard tabs
Column: 1
(MD010, no-hard-tabs)
🔇 Additional comments (1)
hyperfleet/standards/dependency-pinning.md (1)
410-411: All referenced standards documents (linting.md,generated-code-policy.md, andmakefile-conventions.md) exist at the expected paths inhyperfleet/standards/. No action required.
|
|
||
| Every repository using Bingo MUST follow this structure: | ||
|
|
||
| ``` |
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.
Add language specification to code block for syntax highlighting.
Line 34 contains a fenced code block without a language identifier, which affects readability and syntax highlighting in rendered documentation.
🔧 Proposed fix
-```
+```
repo-root/
├── .bingo/Replace with:
-```
+```text
repo-root/
├── .bingo/📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ``` |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
34-34: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In @hyperfleet/standards/dependency-pinning.md at line 34, The fenced code block
at the start of the example (the ``` fence) lacks a language identifier; update
that fence to include a language specifier (use "text") so the block becomes
```text to enable proper syntax highlighting and improve readability.
| @echo "Installing Bingo tools..." | ||
| bingo get github.com/golangci/golangci-lint/cmd/golangci-lint@v2.1.6 | ||
| bingo get github.com/golang/mock/mockgen@v1.6.0 | ||
| bingo get github.com/deepmap/oapi-codegen/cmd/oapi-codegen@v1.12.4 | ||
| @echo "Tools installed successfully!" | ||
|
|
||
| # List installed tools | ||
| .PHONY: tools-list | ||
| tools-list: | ||
| @bingo list | ||
|
|
||
| # Update all tools to latest versions | ||
| .PHONY: tools-update | ||
| tools-update: | ||
| @echo "Updating tools to latest versions..." | ||
| bingo get github.com/golangci/golangci-lint/cmd/golangci-lint | ||
| bingo get github.com/golang/mock/mockgen | ||
| bingo get github.com/deepmap/oapi-codegen/cmd/oapi-codegen | ||
| @echo "Tools updated!" | ||
|
|
||
| # Use tools in standard targets | ||
| .PHONY: lint | ||
| lint: $(GOLANGCI_LINT) | ||
| $(GOLANGCI_LINT) run ./... | ||
|
|
||
| .PHONY: generate | ||
| generate: $(MOCKGEN) $(OAPI_CODEGEN) | ||
| go generate ./... |
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.
Replace hard tabs with spaces in Makefile code blocks for consistency.
Markdownlint detects hard tabs (MD010) in multiple Makefile sections (lines 238–242, 247, 252–256, 261, 265, 286). While Makefiles require tabs for indentation, within documentation code blocks these should be spaces to maintain consistency with rendering and avoid editor confusion.
🔧 Proposed fix (spaces instead of tabs in code blocks)
.PHONY: tools-install
tools-install:
- @echo "Installing Bingo tools..."
- bingo get github.com/golangci/golangci-lint/cmd/golangci-lint@v2.1.6
- bingo get github.com/golang/mock/mockgen@v1.6.0
- bingo get github.com/deepmap/oapi-codegen/cmd/oapi-codegen@v1.12.4
- @echo "Tools installed successfully!"
+ @echo "Installing Bingo tools..."
+ bingo get github.com/golangci/golangci-lint/cmd/golangci-lint@v2.1.6
+ bingo get github.com/golang/mock/mockgen@v1.6.0
+ bingo get github.com/deepmap/oapi-codegen/cmd/oapi-codegen@v1.12.4
+ @echo "Tools installed successfully!"
.PHONY: tools-list
tools-list:
- @bingo list
+ @bingo list(Repeat for all affected Makefile blocks at lines 247, 252–256, 261, 265, 286.)
Also applies to: 286-286
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
238-238: Hard tabs
Column: 1
(MD010, no-hard-tabs)
239-239: Hard tabs
Column: 1
(MD010, no-hard-tabs)
240-240: Hard tabs
Column: 1
(MD010, no-hard-tabs)
241-241: Hard tabs
Column: 1
(MD010, no-hard-tabs)
242-242: Hard tabs
Column: 1
(MD010, no-hard-tabs)
247-247: Hard tabs
Column: 1
(MD010, no-hard-tabs)
252-252: Hard tabs
Column: 1
(MD010, no-hard-tabs)
253-253: Hard tabs
Column: 1
(MD010, no-hard-tabs)
254-254: Hard tabs
Column: 1
(MD010, no-hard-tabs)
255-255: Hard tabs
Column: 1
(MD010, no-hard-tabs)
256-256: Hard tabs
Column: 1
(MD010, no-hard-tabs)
261-261: Hard tabs
Column: 1
(MD010, no-hard-tabs)
265-265: Hard tabs
Column: 1
(MD010, no-hard-tabs)
🤖 Prompt for AI Agents
In @hyperfleet/standards/dependency-pinning.md around lines 238 - 265, The
Makefile examples in the markdown use hard tabs inside the fenced code blocks
(e.g., recipe lines under targets like tools-list, tools-update, lint, generate
and variable references $(GOLANGCI_LINT), $(MOCKGEN), $(OAPI_CODEGEN)); replace
those hard tabs with spaces so the documentation code blocks use only spaces for
indentation (or reformat the blocks to use triple-backtick fenced blocks with
space-indented lines) while keeping the actual Makefile semantics intact in real
files—update every recipe line and indented line in those examples to use spaces
instead of \t.
| | Use Case | Tool | Command | | ||
| |----------|------|---------| | ||
| | **Linting** | golangci-lint | `bingo get github.com/golangci/golangci-lint/cmd/golangci-lint@v2.1.6` | | ||
| | **Mocks** | mockgen | `bingo get github.com/golang/mock/mockgen@v1.6.0` | |
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 github.com/golang/mock repository has been archived and is no longer maintained. The project was migrated to go.uber.org/mock.
Consider updating all mockgen references in this document:
- Current:
bingo get github.com/golang/mock/mockgen@v1.6.0 - Suggested:
bingo get go.uber.org/mock/mockgen@v0.5.0
This affects lines 172, 232, 234, and 254.
Reference: https://github.com/golang/mock#gomock
| │ ├── mockgen-v1.6.0 # Compiled binary (gitignored) | ||
| │ │ | ||
| │ └── oapi-codegen.mod # Tool version specification | ||
| │ oapi-codegen.sum # Dependency checksums |
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.
Being picky...
| │ oapi-codegen.sum # Dependency checksums | |
| ├── oapi-codegen.sum # Dependency checksums |
| | **Mocks** | mockgen | `bingo get github.com/golang/mock/mockgen@v1.6.0` | | ||
| | **OpenAPI** | oapi-codegen | `bingo get github.com/deepmap/oapi-codegen/cmd/oapi-codegen@v1.12.4` | | ||
| | **Kubernetes CRDs** | controller-gen | `bingo get sigs.k8s.io/controller-tools/cmd/controller-gen@v0.13.0` | | ||
| | **Protobuf** | protoc-gen-go | `bingo get google.golang.org/protobuf/cmd/protoc-gen-go@latest` | |
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.
Using @latest for protobuf and gRPC tools contradicts the document's core message about pinning specific versions for reproducible builds. In a document advocating for explicit version pinning, all examples should demonstrate that practice consistently.
| | **Protobuf** | protoc-gen-go | `bingo get google.golang.org/protobuf/cmd/protoc-gen-go@latest` | | |
| | **Protobuf** | protoc-gen-go | `bingo get google.golang.org/protobuf/cmd/protoc-gen-go@v1.36.5` | |
| |----------|------|---------| | ||
| | **Linting** | golangci-lint | `bingo get github.com/golangci/golangci-lint/cmd/golangci-lint@v2.1.6` | | ||
| | **Mocks** | mockgen | `bingo get github.com/golang/mock/mockgen@v1.6.0` | | ||
| | **OpenAPI** | oapi-codegen | `bingo get github.com/deepmap/oapi-codegen/cmd/oapi-codegen@v1.12.4` | |
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 oapi-codegen project has moved from github.com/deepmap/oapi-codegen to github.com/oapi-codegen/oapi-codegen. The deepmap path still works but points to an older version (v1.x), while the new path has v2.x with breaking changes and improvements.
Consider updating to the new path with the latest v2 version:
| | **OpenAPI** | oapi-codegen | `bingo get github.com/deepmap/oapi-codegen/cmd/oapi-codegen@v1.12.4` | | |
| | **OpenAPI** | oapi-codegen | `bingo get github.com/oapi-codegen/oapi-codegen/v2/cmd/oapi-codegen@v2.4.1` | |
| | **Linting** | golangci-lint | `bingo get github.com/golangci/golangci-lint/cmd/golangci-lint@v2.1.6` | | ||
| | **Mocks** | mockgen | `bingo get github.com/golang/mock/mockgen@v1.6.0` | | ||
| | **OpenAPI** | oapi-codegen | `bingo get github.com/deepmap/oapi-codegen/cmd/oapi-codegen@v1.12.4` | | ||
| | **Kubernetes CRDs** | controller-gen | `bingo get sigs.k8s.io/controller-tools/cmd/controller-gen@v0.13.0` | |
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 controller-gen version v0.13.0 is outdated. Consider updating to a more recent version.
| | **Kubernetes CRDs** | controller-gen | `bingo get sigs.k8s.io/controller-tools/cmd/controller-gen@v0.13.0` | | |
| | **Kubernetes CRDs** | controller-gen | `bingo get sigs.k8s.io/controller-tools/cmd/controller-gen@v0.17.2` | |
Version 0.13.0 was released in mid-2023. Newer versions include important fixes and improvements for Kubernetes 1.29+ compatibility.
|
|
||
| ```gitignore | ||
| # Ignore compiled tool binaries | ||
| *-v* |
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 gitignore pattern *-v* is overly broad and could unintentionally match files that aren't Bingo binaries.
For example, it would match:
my-validator.mod(if someone names a tool "validator")review-v2.md(hypothetical doc file)
Consider using a more specific pattern that matches Bingo's actual binary naming convention (semantic version format):
| *-v* | |
| # Ignore compiled tool binaries (Bingo uses format: toolname-vX.Y.Z) | |
| *-v[0-9]* |
| - uses: actions/cache@v4 | ||
| with: | ||
| path: .bingo | ||
| key: ${{ runner.os }}-bingo-${{ hashFiles('.bingo/*.mod') }} |
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 cache configuration uses runner.os but doesn't account for CPU architecture. Since Bingo compiles Go binaries that are architecture-specific, teams with mixed architectures (e.g., Intel and Apple Silicon Macs, or x64 and ARM Linux runners) would experience cache collisions and binary incompatibility errors.
| key: ${{ runner.os }}-bingo-${{ hashFiles('.bingo/*.mod') }} | |
| key: ${{ runner.os }}-${{ runner.arch }}-bingo-${{ hashFiles('.bingo/*.mod') }} |
This ensures each OS+architecture combination has its own cache with correctly compiled binaries.
| run: make generate | ||
|
|
||
| - name: Check for changes | ||
| run: git diff --exit-code # Fail if generated code is stale |
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 verify-generated CI job fails silently with just git diff --exit-code. When this check fails, developers see a cryptic exit code without understanding what went wrong or how to fix it.
| run: git diff --exit-code # Fail if generated code is stale | |
| run: | | |
| if ! git diff --exit-code; then | |
| echo "::error::Generated code is out of sync. Run 'make generate' locally and commit the changes." | |
| git diff --stat | |
| exit 1 | |
| fi |
This provides a clear error message and shows which files differ, making it easier for developers to understand and fix the issue.
| bingo get github.com/golangci/golangci-lint/cmd/golangci-lint@v2.1.6 | ||
|
|
||
| # 4. Update Makefile | ||
| cat >> Makefile <<'EOF' |
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.
This command multiple times (e.g., following the tutorial again or troubleshooting), it will duplicate the content in the Makefile.
Consider either:
-
Add a warning:
4. Update Makefile (run only once - this appends to the file)
cat >> Makefile <<'EOF' -
Or use a safer approach that checks first:
4. Update Makefile
grep -q '.bingo/Variables.mk' Makefile || cat >> Makefile <<'EOF'
include .bingo/Variables.mk
...
EOF
The same issue applies to lines 136-141 in the "Complete Workflow" section.
| @@ -0,0 +1,412 @@ | |||
| # Dependency and Tool Version Pinning Standard | |||
|
|
|||
| All HyperFleet repositories MUST use [Bingo](https://github.com/bwplotka/bingo) to pin development tool versions for reproducible builds across all environments. | |||
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.
| All HyperFleet repositories MUST use [Bingo](https://github.com/bwplotka/bingo) to pin development tool versions for reproducible builds across all environments. | |
| All HyperFleet repositories **containing Go code** MUST use [Bingo](https://github.com/bwplotka/bingo) to pin development tool versions for reproducible builds across all environments. |
| steps: | ||
| - uses: actions/checkout@v4 | ||
|
|
||
| - uses: actions/setup-go@v5 |
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 CI example hardcodes go-version: '1.23' which can drift from the actual Go version used in the project's go.mod.
Consider using a dynamic approach to keep versions in sync:
| - uses: actions/setup-go@v5 | |
| - uses: actions/setup-go@v5 | |
| with: | |
| go-version-file: 'go.mod' | |
| cache: true |
Defines standard for pinning development tool versions across HyperFleet repositories using Bingo.
What
New Files:
hyperfleet/standards/dependency-pinning.md- Standard specificationCovers:
tools-install,tools-update,tools-listWhy
Problem:
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.