Naming conventions #47
Conversation
WalkthroughThis pull request introduces a new comprehensive naming conventions guide document for the HyperFleet project. The document specifies standardized naming rules across domain concepts, architecture patterns, infrastructure components, configuration, observability, API conventions, database schemas, Kubernetes resources, messaging, repository naming, binaries, Makefile targets, and package/module organization. The guide includes examples and template configurations in multiple formats and references common patterns. No code changes or public API modifications are included. Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 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. Comment |
|
|
||
| | Concept | Code (Type) | URL/Config | Plural | Database Table | | ||
| |---------|-------------|------------|--------|----------------| | ||
| | Cluster | `Cluster` | `cluster` | `clusters` | `clusters` | |
There was a problem hiding this comment.
I really feel, we should be calling it a hosted cluster, following what we are actually provisioning. I really dont like cluster as it leaves too much room for interpretation
There was a problem hiding this comment.
I can see that for us (dev teams, BU) it may have more meaning using HostedCluster, but..
What would make the most sense to customers?
Does it bring value for them to name it HostedCluster instead of Cluster?
There was a problem hiding this comment.
We are just an API, how it gets wrapped and presented by the client is up to the users. I really worry that if we are not strongly locked to what a hosted cluster is we will fall into the same trap as OCM did
|
|
||
| | Component Type | Suffix | Interface Name | Implementation | Package | | ||
| |----------------|--------|----------------|----------------|---------| | ||
| | Service layer | `Service` | `ClusterService` | `sqlClusterService` | `pkg/services/` | |
There was a problem hiding this comment.
Same as above, really feel we need to be pushing on HostedClusterService
| <summary>Complete Table Schema Example</summary> | ||
|
|
||
| ```sql | ||
| CREATE TABLE clusters ( |
There was a problem hiding this comment.
Off topic, but I noticed in the API we are not using Gorm conventions for creating tables and instead using tx.Exec for raw sql, is there are reason for this?
|
Nice work @rh-amarin . Although it's a draft, besides the comments from @ciaranRoche regarding using HostedCluster fo examples, this LTGM. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Fix all issues with AI agents
In `@hyperfleet/docs/naming-conventions.md`:
- Around line 233-234: The broker topic examples use PascalCase (`Cluster`,
`NodePool`) but section 1.1 mandates lowercase in URLs/config; update
naming-conventions.md to remove ambiguity by either (a) changing the example
resource_type values to lowercase (`cluster`, `nodepool`) and updating
`BROKER_TOPIC`/`BROKER_SUBSCRIPTION_ID` examples accordingly, or (b) adding one
clear sentence stating that broker topic names are an explicit exception and
must use PascalCase, referencing the table examples (`Cluster`, `NodePool`,
`BROKER_TOPIC`, `BROKER_SUBSCRIPTION_ID`) so readers know which rule applies;
implement one of these fixes so examples and the "lowercase in config" rule are
consistent.
- Line 148: Update the typo in the sentence "We should standarize in:" by
replacing "standarize" with "standardize" so the line reads "We should
standardize in:"; locate the phrase in the naming-conventions.md content (the
exact text "We should standarize in:") and correct the spelling.
- Around line 354-367: The fenced code block showing the secrets directory tree
lacks a language tag (Markdown lint), so update the triple-backtick fence
surrounding the directory tree to include the text language specifier (i.e.,
change ``` to ```text) so the block is typed; locate the directory-tree fenced
block in naming-conventions.md (the secrets/ tree) and add the language
identifier `text` immediately after the opening backticks.
- Around line 743-747: The fenced code block showing the directory tree (the
block containing "cmd/ └── <service-name>/ └── main.go") lacks a language tag;
update the opening fence to use ```text so the tree is rendered as plain text
(replace the current ``` with ```text for that directory-tree code block).
- Line 739: Change the phrase "Full stack services keep the `hyperfleet-` prefix
(e.g., `hyperfleet-api`)" to use the hyphenated compound adjective "Full-stack
services keep the `hyperfleet-` prefix (e.g., `hyperfleet-api`)" so the compound
modifier is correctly hyphenated; locate the sentence containing "Full stack
services" in the naming-conventions.md content and replace it with "Full-stack
services".
- Around line 665-701: The docs state "use lower case for the CloudEvent type"
but the Field Standards and example show PascalCase
("com.redhat.hyperfleet.Cluster.reconcile"); change the type pattern to use
lowercase resource and action (e.g.,
`com.redhat.hyperfleet.<resource>.<action>`), update the Field Standards entry
for `type` and the CloudEvents JSON example accordingly (replace
`com.redhat.hyperfleet.Cluster.reconcile` with
`com.redhat.hyperfleet.cluster.reconcile`), and ensure the prose and examples
consistently reference the `type` field as lowercase.
- Around line 833-862: The fenced code block displaying the repository directory
tree in naming-conventions.md is missing a language tag; update the
triple-backtick fence that wraps the directory listing to use the text language
(i.e., change ``` to ```text) so the tree is rendered with the correct syntax
highlighting and formatting.
🧹 Nitpick comments (1)
hyperfleet/docs/naming-conventions.md (1)
144-144: Avoid bare URL; use a Markdown link.
Bare URL violates markdownlint and reduces readability.♻️ Suggested edit
-TBC in https://issues.redhat.com/browse/HYPERFLEET-323 +TBC in [HYPERFLEET-323](https://issues.redhat.com/browse/HYPERFLEET-323)
|
|
||
| **Standard**: `HyperFleetLogger` interface across all repositories | ||
|
|
||
| We should standarize in: |
There was a problem hiding this comment.
Fix typo: “standarize” → “standardize”.
This is a visible spelling error in a published guide.
✏️ Suggested edit
-We should standarize in:
+We should standardize in:📝 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.
| We should standarize in: | |
| We should standardize in: |
🧰 Tools
🪛 LanguageTool
[grammar] ~148-~148: Ensure spelling is correct
Context: ...face across all repositories We should standarize in: - Configuration - Output (e.g. JSON...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 Prompt for AI Agents
In `@hyperfleet/docs/naming-conventions.md` at line 148, Update the typo in the
sentence "We should standarize in:" by replacing "standarize" with "standardize"
so the line reads "We should standardize in:"; locate the phrase in the
naming-conventions.md content (the exact text "We should standarize in:") and
correct the spelling.
| | Topic name | `<resource_type>` | `Cluster`, `NodePool` | `BROKER_TOPIC` | | ||
| | Queue/Subscription | `<service>-<resource>-sub` | `adapter-cluster-sub`, `sentinel-cluster-sub` | `BROKER_SUBSCRIPTION_ID` | |
There was a problem hiding this comment.
Clarify topic casing vs “lowercase in config” rule.
Section 1.1 says lowercase in URLs/config, but broker topic examples use Cluster/NodePool (PascalCase). Please state explicitly whether broker topic names are an exception or align them to lowercase to avoid divergent implementations.
🤖 Prompt for AI Agents
In `@hyperfleet/docs/naming-conventions.md` around lines 233 - 234, The broker
topic examples use PascalCase (`Cluster`, `NodePool`) but section 1.1 mandates
lowercase in URLs/config; update naming-conventions.md to remove ambiguity by
either (a) changing the example resource_type values to lowercase (`cluster`,
`nodepool`) and updating `BROKER_TOPIC`/`BROKER_SUBSCRIPTION_ID` examples
accordingly, or (b) adding one clear sentence stating that broker topic names
are an explicit exception and must use PascalCase, referencing the table
examples (`Cluster`, `NodePool`, `BROKER_TOPIC`, `BROKER_SUBSCRIPTION_ID`) so
readers know which rule applies; implement one of these fixes so examples and
the "lowercase in config" rule are consistent.
| ``` | ||
| secrets/ | ||
| ├── db.host | ||
| ├── db.name | ||
| ├── db.user | ||
| ├── db.password | ||
| ├── db.port | ||
| ├── hyperfleet-api-token | ||
| ├── ocm.client-id | ||
| ├── ocm.client-secret | ||
| ├── tls.crt | ||
| ├── tls.key | ||
| └── ca.crt | ||
| ``` |
There was a problem hiding this comment.
Add a language to fenced code block.
Markdownlint flags untyped fences; use text for directory trees.
♻️ Suggested edit
-```
+```text
secrets/
├── db.host
├── db.name
├── db.user
├── db.password
├── db.port
├── hyperfleet-api-token
├── ocm.client-id
├── ocm.client-secret
├── tls.crt
├── tls.key
└── ca.crt</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **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.
```suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
354-354: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In `@hyperfleet/docs/naming-conventions.md` around lines 354 - 367, The fenced
code block showing the secrets directory tree lacks a language tag (Markdown
lint), so update the triple-backtick fence surrounding the directory tree to
include the text language specifier (i.e., change ``` to ```text) so the block
is typed; locate the directory-tree fenced block in naming-conventions.md (the
secrets/ tree) and add the language identifier `text` immediately after the
opening backticks.
| **Standard**: Follow CloudEvents v1.0 specification | ||
|
|
||
| Use lower case for the CloudEvent type, and snake_case for data fields | ||
|
|
||
| <details> | ||
| <summary>CloudEvents JSON Example</summary> | ||
|
|
||
| ```json | ||
| { | ||
| "specversion": "1.0", | ||
| "type": "com.redhat.hyperfleet.cluster.reconcile", | ||
| "source": "hyperfleet/services/sentinel", | ||
| "id": "550e8400-e29b-41d4-a716-446655440000", | ||
| "time": "2025-12-06T12:00:00Z", | ||
| "datacontenttype": "application/json", | ||
| "data": { | ||
| "type": "Cluster", | ||
| "id": "cluster-abc123", | ||
| "href": "/api/v1/clusters/cluster-abc123", | ||
| "generation": 5, | ||
| "reason": "MaxAgeExceeded" | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| </details> | ||
|
|
||
| **Field Standards**: | ||
|
|
||
| | Field | Format | Example | Notes | | ||
| |-------|--------|---------|-------| | ||
| | `type` | `com.redhat.hyperfleet.<Resource>.<action>` | `com.redhat.hyperfleet.Cluster.reconcile` | Reverse DNS + resource + action | | ||
| | `source` | `<service-name>` | `hyperfleet-sentinel`, `dns-adapter` | Service that emitted event | | ||
| | `id` | UUID | `550e8400-e29b-41d4-a716-446655440000` | Unique per event | | ||
| | `time` | RFC3339 | `2025-12-06T12:00:00Z` | UTC timestamp | | ||
|
|
||
| **Data Payload**: Use `snake_case` for field names (`resource_type`, `resource_id`, `href`, not `resourceType`). |
There was a problem hiding this comment.
Resolve CloudEvents type casing inconsistency.
This section says “use lower case for the CloudEvent type,” but the field standard shows com.redhat.hyperfleet.<Resource>.<action> with a PascalCase resource and example com.redhat.hyperfleet.Cluster.reconcile. This conflicts with the stated rule and will lead to incompatible event types across services.
♻️ Suggested edit (align to lowercase type)
-| `type` | `com.redhat.hyperfleet.<Resource>.<action>` | `com.redhat.hyperfleet.Cluster.reconcile` | Reverse DNS + resource + action |
+| `type` | `com.redhat.hyperfleet.<resource>.<action>` | `com.redhat.hyperfleet.cluster.reconcile` | Reverse DNS + resource + action |🤖 Prompt for AI Agents
In `@hyperfleet/docs/naming-conventions.md` around lines 665 - 701, The docs state
"use lower case for the CloudEvent type" but the Field Standards and example
show PascalCase ("com.redhat.hyperfleet.Cluster.reconcile"); change the type
pattern to use lowercase resource and action (e.g.,
`com.redhat.hyperfleet.<resource>.<action>`), update the Field Standards entry
for `type` and the CloudEvents JSON example accordingly (replace
`com.redhat.hyperfleet.Cluster.reconcile` with
`com.redhat.hyperfleet.cluster.reconcile`), and ensure the prose and examples
consistently reference the `type` field as lowercase.
|
|
||
| **Pattern**: | ||
| - Service-level tools use short names (e.g., `sentinel`) | ||
| - Full stack services keep the `hyperfleet-` prefix (e.g., `hyperfleet-api`) |
There was a problem hiding this comment.
Hyphenate compound adjective: “full-stack”.
Use “full-stack services” when modifying a noun.
✏️ Suggested edit
-- Full stack services keep the `hyperfleet-` prefix (e.g., `hyperfleet-api`)
+- Full-stack services keep the `hyperfleet-` prefix (e.g., `hyperfleet-api`)🧰 Tools
🪛 LanguageTool
[uncategorized] ~739-~739: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...ls use short names (e.g., sentinel) - Full stack services keep the hyperfleet- prefix ...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
🤖 Prompt for AI Agents
In `@hyperfleet/docs/naming-conventions.md` at line 739, Change the phrase "Full
stack services keep the `hyperfleet-` prefix (e.g., `hyperfleet-api`)" to use
the hyphenated compound adjective "Full-stack services keep the `hyperfleet-`
prefix (e.g., `hyperfleet-api`)" so the compound modifier is correctly
hyphenated; locate the sentence containing "Full stack services" in the
naming-conventions.md content and replace it with "Full-stack services".
| ``` | ||
| cmd/ | ||
| └── <service-name>/ # e.g., sentinel, adapter, hyperfleet-api | ||
| └── main.go | ||
| ``` |
There was a problem hiding this comment.
Add a language to fenced code block.
Use text for directory trees.
♻️ Suggested edit
-```
+```text
cmd/
└── <service-name>/ # e.g., sentinel, adapter, hyperfleet-api
└── main.go</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **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.
```suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
743-743: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In `@hyperfleet/docs/naming-conventions.md` around lines 743 - 747, The fenced
code block showing the directory tree (the block containing "cmd/ └──
<service-name>/ └── main.go") lacks a language tag; update the opening fence to
use ```text so the tree is rendered as plain text (replace the current ``` with
```text for that directory-tree code block).
| ``` | ||
| <repository>/ | ||
| ├── cmd/ # Application entry points | ||
| │ └── <service>/ | ||
| │ └── main.go | ||
| ├── pkg/ # Public packages (importable) | ||
| │ ├── api/ # API models | ||
| │ ├── logger/ # Logging | ||
| │ ├── errors/ # Error types | ||
| │ └── ... | ||
| ├── internal/ # Private packages (not importable) | ||
| │ ├── <component>/ | ||
| │ └── ... | ||
| ├── test/ # Integration tests | ||
| │ └── integration/ | ||
| ├── configs/ # Configuration examples | ||
| ├── charts/ # Helm charts | ||
| │ └── <service>/ | ||
| ├── deployments/ # Deployment artifacts (optional) | ||
| │ ├── helm/ | ||
| │ └── dashboards/ | ||
| ├── docs/ # Documentation | ||
| ├── scripts/ # Build/deployment scripts | ||
| ├── .github/ # GitHub Actions workflows | ||
| ├── Dockerfile | ||
| ├── Makefile | ||
| ├── go.mod | ||
| ├── go.sum | ||
| └── README.md | ||
| ``` |
There was a problem hiding this comment.
Add a language to fenced code block.
Use text for directory layout trees.
♻️ Suggested edit
-```
+```text
<repository>/
├── cmd/ # Application entry points
│ └── <service>/
│ └── main.go
├── pkg/ # Public packages (importable)
│ ├── api/ # API models
│ ├── logger/ # Logging
│ ├── errors/ # Error types
│ └── ...
├── internal/ # Private packages (not importable)
│ ├── <component>/
│ └── ...
├── test/ # Integration tests
│ └── integration/
├── configs/ # Configuration examples
├── charts/ # Helm charts
│ └── <service>/
├── deployments/ # Deployment artifacts (optional)
│ ├── helm/
│ └── dashboards/
├── docs/ # Documentation
├── scripts/ # Build/deployment scripts
├── .github/ # GitHub Actions workflows
├── Dockerfile
├── Makefile
├── go.mod
├── go.sum
└── README.md</details>
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.18.1)</summary>
833-833: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
In @hyperfleet/docs/naming-conventions.md around lines 833 - 862, The fenced
code block displaying the repository directory tree in naming-conventions.md is
missing a language tag; update the triple-backtick fence that wraps the
directory listing to use the text language (i.e., change totext) so the
tree is rendered with the correct syntax highlighting and formatting.
</details>
<!-- fingerprinting:phantom:triton:eagle -->
<!-- This is an auto-generated comment by CodeRabbit -->
3 participants
This PR documents how to name different concerns for the different Hyperfleet components.
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.