-
Notifications
You must be signed in to change notification settings - Fork 13
HYPERFLEET-442: Add repository type variations to Makefile conventions #68
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
Conversation
WalkthroughThis PR updates the Makefile conventions doc by adding a new "Repository Type Variations" section, introducing repository types (Service, Helm-chart, Infrastructure, Documentation), and documenting a root .hyperfleet.yaml repository-type indicator plus auto-detection fallback rules. It expands target mappings (including helm-chart equivalents and renaming helm-test), updates audit semantics, reorders the TOC (moving Flag Conventions), and adds multi-type examples and Makefile/YAML snippets. All changes are documentation-only. Sequence Diagram(s)(omitted) Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🔇 Additional comments (5)
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: 1
🤖 Fix all issues with AI agents
In @hyperfleet/standards/makefile-conventions.md:
- Line 211: The fenced code block in the audit tool example output is missing a
language identifier; update the triple-backtick block that currently contains
"Repository: adapter-landing-zone\n Type: helm-chart (from .hyperfleet.yaml)" to
include a language specifier (e.g., change ``` to ```plaintext) so the markdown
linter requirement for a fenced code block language is satisfied.
🧹 Nitpick comments (1)
hyperfleet/standards/makefile-conventions.md (1)
203-207: Auto-detection fallback logic could benefit from priority rules for edge cases.The fallback detection lists signals for each type (e.g.,
charts/→helm-chart,go.mod→service) but does not specify behavior when multiple markers are present in the same repository. For example:
- A repo with both
go.modandcharts/directories (e.g., a service with embedded Helm charts)- A repo with both
terraform/and.mdfilesConsider documenting detection priority rules or recommending that such repos always include an explicit
.hyperfleet.yamlto avoid ambiguity.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
hyperfleet/standards/makefile-conventions.md
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
hyperfleet/standards/makefile-conventions.md
211-211: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (4)
hyperfleet/standards/makefile-conventions.md (4)
12-14: Table of Contents updates align well with restructured sections.The reordering to insert "Repository Type Variations" between standard targets and flag conventions matches the document flow and makes good logical sense.
81-81: Helm-related target naming standardized consistently.The rename from
test-helmtohelm-testis applied across the optional targets table and examples, maintaining consistenthelm-*naming conventions throughout.Also applies to: 83-83, 96-96
154-186: Repository type variations are clearly documented with helpful examples and metadata format.The section provides concrete definitions of each repository type, an intuitive
.hyperfleet.yamlformat for explicit typing, and realistic examples. The structure is well-organized and should guide repository maintainers effectively.
192-195: Clarify ifhelm-lintandhelm-templateare redundant as separate required targets.The required targets for Helm-chart repos list
helm-lint,helm-template, andhelm-testall as required. However, the target equivalents table (line 170) states that the standardtesttarget maps tohelm-test, which in turn runs bothhelm-lintandhelm-templatevalidation. Ifhelm-testis a composite target that already invokes both, there's potential redundancy or confusion about whether the underlying lint/template targets must also be independently required.Do you intend
helm-testto be a composite convenience target, with the underlying lint/template targets as optional building blocks? Or shouldhelm-testbe the only test-related required target (with lint/template being optional helpers)? Clarifying this distinction will help maintainers and auditors understand expectations more precisely.
- Add Repository Type Variations section documenting different repo types - Define target equivalents for Helm-chart repositories (helm-lint, helm-template, helm-test) - Rename test-helm to helm-test for consistent naming pattern - Document .hyperfleet.yaml as repository type indicator - Add audit tool behavior for recognizing repository patterns - Documentation repos do not require Makefile
58141fc to
6b06daa
Compare
|
|
||
| | Type | Description | Examples | | ||
| |------|-------------|----------| | ||
| | **Service** | Repositories that compile Go binaries | adapter-pullsecret, adapter-dns, sentinel | |
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.
adapter-dns doesn't exist (yet), but it would be similar to landing-zone and will not contain a binary (IIRC)
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.
Ok. Removed
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.
Did I spoke too fast?
Re-thinking on it.... validators that provide a job, will build some sort of binary...
Right now our adapter jobs are just scripts, so the job they build don't require building a specific container image.
But there may be others that do.... in that case... will the repositories be service or helm type?
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.
Good point! I've updated the documentation to support multiple types. If a repository builds binaries AND contains Helm charts, it can declare both:
types: [service, helm-chart]
The audit tool will validate required targets for each type. This way you don't have to choose - hybrid repos are explicitly supported. See the new "Example: Service repository with Helm charts" section.
|
|
||
| | Standard Target | Helm-chart Equivalent | Notes | | ||
| |-----------------|----------------------|-------| | ||
| | `build` | N/A | No binaries to build; target may be omitted | |
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.
It may be equivalent to helm template, since descriptors are the only thing being "built"
In some companies, the helm templates get rendered and stored in git and then deployed, so they can be considered as the "build output" of the repo
If this is not convincing, then better to omit it always and not say "target may be omited" since now we have to decide in every repo what to do
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.
Good point. I've updated to make it explicit that build is not applicable for Helm-chart repositories (removed the ambiguous "may be omitted" wording). Since helm-template validation is already included in helm-test, mapping it to build would be redundant
|
|
||
| ### Repository Type Indicator | ||
|
|
||
| To help tooling identify repository types, repositories **SHOULD** include a `.hyperfleet.yaml` file in the root directory: |
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.
Isn't this something already provided by GitHub?
For "type" we can use the Topic property that you can set to a repository.
And then, the github API allows to query this info.
The description is already there in our repos
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.
Good question! I considered GitHub Topics, but .hyperfleet.yaml is preferred for a few reasons:
- Works offline - the standards-audit tool runs locally on cloned repos without needing GitHub API calls
- Version-controlled - changes go through PR review, unlike Topics which can be changed by any admin
- Structured metadata - allows for future expansion (owner, team, etc.)
- Platform-agnostic - works regardless of git hosting provider
GitHub Topics are still useful for discovery/cataloging at the GitHub level, but for local tooling the file approach is more reliable. I've added a note explaining this in the document.
| # .hyperfleet.yaml - Repository metadata for HyperFleet tooling | ||
| version: v1 | ||
| repository: | ||
| type: helm-chart # One of: service, helm-chart, infrastructure, documentation |
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.
I am wondering what does type mean here? main code type or deployment type or running type or usage type?
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.
Good point! The type refers to the repository build artifact type - what kind of output the repository produces (binary, Helm charts, infrastructure code, or documentation). I've updated the comment to clarify this and reference the Repository Types table that defines each type.
- Remove adapter-dns from Service examples (doesn't exist yet) - Clarify build target is not applicable for Helm-chart repos - Explain why .hyperfleet.yaml is preferred over GitHub Topics - Improve type field comment to clarify it refers to build artifact type Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Rename 'type' to 'types' (always a list) to support hybrid repositories - Add example of service repository with Helm charts - Clarify that targets are additive and each type requires its own targets - Update audit output example to use 'Types:' format Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
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: 1
🤖 Fix all issues with AI agents
In @hyperfleet/standards/makefile-conventions.md:
- Line 212: Replace the pseudo-headings that use bold emphasis (the "**...**"
subsection labels flagged by the linter) with proper markdown heading syntax by
changing each bold label to a level-3 heading (### followed by the same label
text), and ensure there is a blank line before and after each new heading to
satisfy markdown structure and the linter.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
hyperfleet/standards/makefile-conventions.md
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
hyperfleet/standards/makefile-conventions.md
211-211: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🔇 Additional comments (1)
hyperfleet/standards/makefile-conventions.md (1)
150-257: New Repository Type Variations section is well-structured and comprehensive.The new section provides clear guidance on repository-type variations with well-organized subsections: repository types with descriptions, Helm-chart target equivalents,
.hyperfleet.yamlmetadata format, audit tool behavior, and practical multi-type examples. The renaming oftest-helmtohelm-testis consistent throughout the updated tables and examples.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
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: 1
🤖 Fix all issues with AI agents
In @hyperfleet/standards/makefile-conventions.md:
- Line 65: Two identical headings "Example invocation" cause an MD024 lint
error; rename one (or both) headings so they are unique and reflect their
context (for example "Example invocation — CLI" and "Example invocation —
Makefile" or "Usage example"). Update both header lines that read "Example
invocation" to distinct, descriptive titles and ensure any internal
links/anchors referencing them are updated accordingly.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
hyperfleet/standards/makefile-conventions.md
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
hyperfleet/standards/makefile-conventions.md
92-92: Multiple headings with the same content
(MD024, no-duplicate-heading)
🔇 Additional comments (2)
hyperfleet/standards/makefile-conventions.md (2)
75-101: Consistent helm-test naming convention applied throughout.The renaming from
test-helmtohelm-testcorrectly follows thehelm-*pattern and aligns with Helm CLI conventions. Changes are consistently applied across the description (line 84), example (line 98), and optional targets table.
152-258: Comprehensive new Repository Type Variations section successfully documents all acceptance criteria.This section excellently implements the PR objectives:
- Repository types (Service, Helm-chart, Infrastructure, Documentation) are clearly defined with examples
- Target equivalents for Helm-chart repos are thoroughly documented with the helm-test, helm-lint, helm-template pattern
.hyperfleet.yamlmetadata indicator is well-explained with practical YAML example- Audit tool behavior explicitly documents compliance detection with sensible auto-detection fallback (charts/ → helm-chart, go.mod → service, etc.)
- Target additivity principle is clearly stated (line 211)
- Real-world examples cover single-type and multi-type repositories
- Audit output example demonstrates expected compliance reporting format
- Rename 'Example invocation' to 'Example: Required targets' - Rename second 'Example invocation' to 'Example: Optional targets' Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary
Updates the Makefile Conventions standard to recognize Helm-chart repository patterns as valid alternatives to the base required targets.
Changes
helm-lint,helm-template,helm-test)test-helmtohelm-testfor consistenthelm-*pattern.hyperfleet.yamlfile for tooling to identify repo typesAcceptance Criteria
Related
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.