-
Notifications
You must be signed in to change notification settings - Fork 98
[docs] Tenants cannot have dashes in the names #980
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
Gave examples of tenant naming. Part of #971 Signed-off-by: Nick Volynkin <nick.volynkin@gmail.com>
* Brush up some formatting * Explain the relations of nested tenants in more detail Signed-off-by: Nick Volynkin <nick.volynkin@gmail.com>
WalkthroughThe tenant documentation was reorganized and expanded for clarity. New sections were added to specify tenant naming rules, particularly disallowing dashes. Existing sections were renamed and reformatted to clarify domain inheritance, tenant nesting, and service reuse, with additional examples and explanations provided. Changes
Possibly related issues
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 0
🧹 Nitpick comments (7)
packages/apps/tenant/README.md (7)
7-11
: Heading level and clarity in "Tenant naming" section
The heading jumps from h1 to h3; it should be an h2 (## Tenant naming
) to follow a single-level increment.
Consider also specifying the allowed name pattern explicitly (e.g., matching^[A-Za-z0-9]+$
) for clarity.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
7-7: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3(MD001, heading-increment)
15-17
: Typographical refinements in examples
- Add a comma after “internally” in line 15 for better readability.
- Use “cannot” (one word) instead of “can not” in line 17, and remove the comma before “because” since the clause is essential.
🧰 Tools
🪛 LanguageTool
[typographical] ~15-~15: Consider adding a comma after ‘internally’ for more clarity.
Context: ... The root tenant is namedroot
, but internally it's referenced astenant-root
. - A...(RB_LY_COMMA)
[style] ~17-~17: Unless you want to emphasize “not”, use “cannot” which is more common.
Context: ...e names and URLs. - However, a tenant can not be namedfoo-bar
, because parsing nam...(CAN_NOT_PREMIUM)
[formatting] ~17-~17: If the ‘because’ clause is essential to the meaning, do not use a comma before the clause.
Context: ...ver, a tenant can not be namedfoo-bar
, because parsing names such astenant-foo-bar
...(COMMA_BEFORE_BECAUSE)
19-19
: Consistent heading levels for "Unique domains"
Change this h3 to h2 (## Unique domains
) so it aligns with the other top-level sections under# Tenant
.
28-32
: Remove trailing whitespace from code fence
The opening fence (```text
) has trailing spaces—trim it totext
and ensure the closing fence has no extra indentation.
34-37
: Consistent heading levels for nesting section
Change this h3 to h2 (## Nesting tenants and reusing parent services
) to maintain a single-level heading hierarchy beneath# Tenant
.
39-40
: Grammar improvement for parent services reference
Line 40 reads “it can access ones of its parent.” Consider rephrasing to “it can access those of its parent” for clarity.
48-50
: Refactor wording for conciseness
Consider combining lines 48–50 into a single sentence for readability, e.g.:
“Sincetenant-u2
doesn’t deploy its own etcd, ingress, or monitoring services, its applications use the parent tenant’s cluster services.”
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/apps/tenant/README.md
(1 hunks)
🧰 Additional context used
🪛 LanguageTool
packages/apps/tenant/README.md
[typographical] ~15-~15: Consider adding a comma after ‘internally’ for more clarity.
Context: ... The root tenant is named root
, but internally it's referenced as tenant-root
. - A...
(RB_LY_COMMA)
[style] ~17-~17: Unless you want to emphasize “not”, use “cannot” which is more common.
Context: ...e names and URLs. - However, a tenant can not be named foo-bar
, because parsing nam...
(CAN_NOT_PREMIUM)
[formatting] ~17-~17: If the ‘because’ clause is essential to the meaning, do not use a comma before the clause.
Context: ...ver, a tenant can not be named foo-bar
, because parsing names such as tenant-foo-bar
...
(COMMA_BEFORE_BECAUSE)
🪛 markdownlint-cli2 (0.17.2)
packages/apps/tenant/README.md
7-7: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build and Test
🔇 Additional comments (3)
packages/apps/tenant/README.md (3)
42-44
: Approve creation example items
The bullet items clearly illustrate tenant creation and service setup; no changes needed.
51-51
: Approve "This in turn means" introduction
The transition phrase works well to lead into the following list.
53-55
: Approve details on shared parent services
The bullet points are clear and accurate regarding etcd storage, ingress access, and monitoring metrics.
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.
LGTM
Summary by CodeRabbit