HYPERFLEET-377: Add Makefile conventions standard #55
Conversation
WalkthroughAdds hyperfleet/docs/makefile-conventions.md, a documentation-only file that specifies standardized Makefile targets and conventions for HyperFleet repositories. It enumerates required targets (help, build, test, lint, clean) and optional targets (test-integration, image, image-push, helm-lint, helm-template, deploy, run); target naming rules; binary output ( Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Suggested reviewers
Pre-merge checks✅ Passed checks (3 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
hyperfleet/docs/makefile-conventions.md (4)
113-131: Specify language identifier for code block (MD040).Line 113 shows a directory structure example without a language identifier. Add
textoryamlidentifier to the opening fence for consistency and to satisfy linting requirements.-``` +```text repo-root/ ├── bin/ # Compiled binaries (gitignored)
138-254: Hard tabs in Makefile code blocks are intentional (MD010 linter note).The markdownlint static analysis flags hard tabs (lines 141, 145, 191–192, 226–231, 237–238, 243–244, 247–248, 253–254, 264, 268) as violating MD010. However, these tabs are necessary and correct in Makefile syntax—commands under targets must use literal tab indentation, not spaces.
To silence the linter while keeping the correct syntax, add a language identifier
makefileto each Makefile code block. This tells linters to treat tabs as required and correct.🔎 Example fix for one code block
-```makefile +```makefile # Good - output to bin/ directory # Example: go build -o bin/pull-secret ./cmd/pull-secret build: - go build -o bin/app-name ./cmd/app-name + go build -o bin/app-name ./cmd/app-name(Note: The indentation here is already
makefilelanguage, so the linter should not flag it when the language is specified.)
288-304: Specify language identifier for help output code block (MD040).Line 288 shows an example help output without a language identifier. Add
bashortextidentifier for consistency.-``` +```text my-service - Available targets Version: v1.0.0
1-396: Comprehensive and well-structured documentation.This guide is thorough and practical. It clearly establishes standardized conventions, provides concrete examples, and includes a compliance checklist for verification. The before/after comparison (lines 359–392) effectively illustrates the value of standardization.
Recommendations for future enhancement:
- Consider adding a section on cross-platform considerations (Windows paths, shell differences).
- If repos use CI/CD systems (GitHub Actions, GitLab CI, etc.), include integration examples showing how standard targets fit into pipelines.
- Consider guidance on secret management if the
image-pushordeploytargets involve credentials.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
hyperfleet/docs/makefile-conventions.md
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
hyperfleet/docs/makefile-conventions.md
113-113: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
141-141: Hard tabs
Column: 1
(MD010, no-hard-tabs)
145-145: Hard tabs
Column: 1
(MD010, no-hard-tabs)
191-191: Hard tabs
Column: 1
(MD010, no-hard-tabs)
192-192: Hard tabs
Column: 1
(MD010, no-hard-tabs)
226-226: Hard tabs
Column: 1
(MD010, no-hard-tabs)
227-227: Hard tabs
Column: 1
(MD010, no-hard-tabs)
228-228: Hard tabs
Column: 1
(MD010, no-hard-tabs)
229-229: Hard tabs
Column: 1
(MD010, no-hard-tabs)
230-230: Hard tabs
Column: 1
(MD010, no-hard-tabs)
231-231: Hard tabs
Column: 1
(MD010, no-hard-tabs)
237-237: Hard tabs
Column: 1
(MD010, no-hard-tabs)
238-238: Hard tabs
Column: 1
(MD010, no-hard-tabs)
243-243: Hard tabs
Column: 1
(MD010, no-hard-tabs)
244-244: Hard tabs
Column: 1
(MD010, no-hard-tabs)
247-247: Hard tabs
Column: 1
(MD010, no-hard-tabs)
248-248: 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)
264-264: Hard tabs
Column: 1
(MD010, no-hard-tabs)
268-268: Hard tabs
Column: 1
(MD010, no-hard-tabs)
288-288: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
|
|
||
| | Target | Description | When to Use | Example | | ||
| |--------|-------------|-------------|---------| | ||
| | `integration-test` | Run integration tests | If repo has integration tests requiring external dependencies | Tests against real GCP/K8s | |
There was a problem hiding this comment.
I would suggest changing to test-integration so an autocomplete can show all test* targets.
There was a problem hiding this comment.
I've updated the ticket to use test-integration instead.
|
|
||
| --- | ||
|
|
||
| ## Directory Structure |
There was a problem hiding this comment.
Although this is ouf of scope of ticket, I think it's important to have this standard. I'm just not sure if it should be part of this document.
Maybe it would be a a new project-structure.md document. WDYT? I've opened https://issues.redhat.com/browse/HYPERFLEET-411 and assigned it to you to make it easy.
|
|
||
| --- | ||
|
|
||
| ## Examples |
There was a problem hiding this comment.
I would completely remove this section as the Standard Targets section already shows this.
|
|
||
| --- | ||
|
|
||
| ## Compliance Checklist |
There was a problem hiding this comment.
Isn't the document itself already a compliance check? I would remove this section to reduce the "complexity/size" of this document.
There was a problem hiding this comment.
Makes sense! Done!
|
|
||
| --- | ||
|
|
||
| ## Help Target Format |
There was a problem hiding this comment.
We are not including implementation details in the documents. Just the spec. The objective is keep the document in a high-level format. So I would suggest removing this section to reduce the "complexity/size" of it.
rafabene
changed the title
|
Please, add the ticket id [HYPERFLEET-377] to the commit message. Tks |
|
I also have updated ticket to define the |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
hyperfleet/docs/makefile-conventions.md
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
hyperfleet/docs/makefile-conventions.md
112-112: Hard tabs
Column: 1
(MD010, no-hard-tabs)
116-116: Hard tabs
Column: 1
(MD010, no-hard-tabs)
162-162: Hard tabs
Column: 1
(MD010, no-hard-tabs)
163-163: Hard tabs
Column: 1
(MD010, no-hard-tabs)
🔇 Additional comments (2)
hyperfleet/docs/makefile-conventions.md (2)
73-93: Verify whethermake generatetarget should be included.Per the PR objectives, there was discussion about defining a
make generatetarget to align with PR #57 and HCM project conventions (for generating OpenAPI modules). The optional targets section doesn't currently mentiongenerate. Please clarify whether this target should be added to the optional targets list along with a description of its purpose.
1-187: Well-structured and comprehensive documentation.The document is clear, well-organized, and effectively addresses the standardization goals. It follows the guidance from previous reviews by remaining spec-focused and concise. The examples are practical and the naming conventions are well-justified.
| go build -o bin/app-name ./cmd/app-name | ||
|
|
||
| # Bad - DO NOT output to project root | ||
| build: | ||
| go build -o app-name ./cmd/app-name |
There was a problem hiding this comment.
Fix hard tabs in code block examples.
The markdown code blocks contain hard tab characters (detected by markdownlint MD010). Use spaces instead for consistency with markdown best practices and to ensure proper rendering across different platforms.
🔎 Proposed fix: Replace tabs with spaces in code blocks
build:
- go build -o bin/app-name ./cmd/app-name
+ go build -o bin/app-name ./cmd/app-name
# Bad - DO NOT output to project root
build:
- go build -o app-name ./cmd/app-name
+ go build -o app-name ./cmd/app-name
build:
- $(Q)echo "Building..."
- $(Q)go build $(GO_FLAGS) -o bin/app-name ./cmd/app-name
+ $(Q)echo "Building..."
+ $(Q)go build $(GO_FLAGS) -o bin/app-name ./cmd/app-nameAlso applies to: 162-163
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
112-112: Hard tabs
Column: 1
(MD010, no-hard-tabs)
116-116: Hard tabs
Column: 1
(MD010, no-hard-tabs)
🤖 Prompt for AI Agents
In hyperfleet/docs/makefile-conventions.md around lines 112-116 (and also lines
162-163), code block examples contain hard tab characters; replace those tabs
with spaces so the code fences use only spaces for indentation (e.g., convert
each tab to 2 or 4 spaces consistent with the document), and verify there are no
remaining tab characters in fenced code blocks before committing.
…nd comprehensive testing
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
hyperfleet/docs/makefile-conventions.md (1)
113-122: Fix hard tab characters in makefile code examples.The markdown code blocks on lines 117 and 121 contain hard tab characters instead of spaces, which violates the MD010 linter rule (also flagged in previous reviews). Replace all tabs with spaces in the code fences for consistency.
🔎 Proposed fix: Replace hard tabs with spaces
```makefile # Good - output to bin/ directory # Example: go build -o bin/pull-secret ./cmd/pull-secret build: - go build -o bin/app-name ./cmd/app-name + go build -o bin/app-name ./cmd/app-name # Bad - DO NOT output to project root build: - go build -o app-name ./cmd/app-name + go build -o app-name ./cmd/app-name</details> </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used**: Organization UI **Review profile**: CHILL **Plan**: Pro <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 364802c3491be96a410475dd069f776a72dd650d and 10c9e01d117b1041ef014ddb23eddcfc80df041f. </details> <details> <summary>📒 Files selected for processing (1)</summary> * `hyperfleet/docs/makefile-conventions.md` </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🪛 markdownlint-cli2 (0.18.1)</summary> <details> <summary>hyperfleet/docs/makefile-conventions.md</summary> 117-117: Hard tabs Column: 1 (MD010, no-hard-tabs) --- 121-121: Hard tabs Column: 1 (MD010, no-hard-tabs) --- 179-179: Hard tabs Column: 1 (MD010, no-hard-tabs) --- 180-180: Hard tabs Column: 1 (MD010, no-hard-tabs) </details> </details> </details> <details> <summary>🔇 Additional comments (3)</summary><blockquote> <details> <summary>hyperfleet/docs/makefile-conventions.md (3)</summary><blockquote> `50-98`: **Excellent structure and comprehensiveness.** The standard targets section clearly distinguishes between required and optional targets, includes practical examples, and provides good guidance on naming conventions. The additions of `test-all`, `test-helm`, and the `generate` target are well-justified and address team feedback from previous iterations. --- `79-79`: **Verify `make generate` specification aligns with PR #57.** Per the PR objectives, the `make generate` target specification should coordinate with changes proposed in PR #57 to ensure consistent behavior across related PRs. Confirm that the brief description on line 79 matches the expected implementation details from that PR. --- `124-145`: **Well-documented temporary files and gitignore guidance.** Clear guidance on artifact locations (bin/, build/) and a practical gitignore template directly addresses the concern raised in previous reviews. This will help teams avoid committing generated files and coverage reports. </blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| ```makefile | ||
| VERBOSE ?= 0 | ||
|
|
||
| ifeq ($(VERBOSE),1) | ||
| GO_FLAGS += -v | ||
| Q = | ||
| else | ||
| Q = @ | ||
| endif | ||
|
|
||
| build: | ||
| $(Q)echo "Building..." | ||
| $(Q)go build $(GO_FLAGS) -o bin/app-name ./cmd/app-name | ||
| ``` |
There was a problem hiding this comment.
Fix hard tab characters in boolean flag convention example.
The makefile code block (lines 179–180) contains hard tab characters instead of spaces. Replace tabs with spaces to satisfy MD010 linter.
🔎 Proposed fix: Replace hard tabs with spaces
```makefile
VERBOSE ?= 0
ifeq ($(VERBOSE),1)
GO_FLAGS += -v
Q =
else
Q = @
endif
build:
- $(Q)echo "Building..."
- $(Q)go build $(GO_FLAGS) -o bin/app-name ./cmd/app-name
+ $(Q)echo "Building..."
+ $(Q)go build $(GO_FLAGS) -o bin/app-name ./cmd/app-name</details>
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.18.1)</summary>
179-179: Hard tabs
Column: 1
(MD010, no-hard-tabs)
---
180-180: Hard tabs
Column: 1
(MD010, no-hard-tabs)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
In hyperfleet/docs/makefile-conventions.md around lines 168 to 181, the makefile
example contains hard tab characters on the two command lines (the build recipe)
which triggers the MD010 linter; replace the hard tabs with spaces in those
lines so the fenced code block uses spaces for indentation (preserve the same
visual indentation and content, only convert tabs to spaces) and re-run the
linter to confirm the MD010 warning is resolved.
</details>
<!-- fingerprinting:phantom:triton:mongoose -->
<!-- This is an auto-generated comment by CodeRabbit -->
|
/lgtm |
3 participants
Standardize Makefile targets and directory structure across all HyperFleet repositories.
Related to HYPERFLEET-377
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.