-
Notifications
You must be signed in to change notification settings - Fork 173
[ECH] Add description of max node count 32 #3686
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
🔍 Preview links for changed docs |
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. TIL that the API doesn't have this limit 🎉
|
@jakommo thank you for the review! ❤️ @elastic/admin-docs could you also kindly help review from docs team's perspective please? Thank you! |
added an example per internal sync
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! Added small suggestions for capitalization and one sentence structure.
In general this doc would need a review around using lists in some sections directly without any kind of introduction.
For example I don't like much how the node count and size new subsection is presented (directly with a bullets), but taking a look at the rest of the document, the style matches :)
In contrast, the way you have introduced the list in the ech specific limitations subsection looks nicer.
But not a big deal! From content point of view all looks good, not sure if anyone else from @elastic/admin-docs might want to add something else.
deploy-manage/deploy/elastic-cloud/restrictions-known-problems.md
Outdated
Show resolved
Hide resolved
deploy-manage/deploy/elastic-cloud/restrictions-known-problems.md
Outdated
Show resolved
Hide resolved
deploy-manage/deploy/elastic-cloud/restrictions-known-problems.md
Outdated
Show resolved
Hide resolved
Co-authored-by: Edu González de la Herrán <25320357+eedugon@users.noreply.github.com>
Co-authored-by: Edu González de la Herrán <25320357+eedugon@users.noreply.github.com>
Co-authored-by: Edu González de la Herrán <25320357+eedugon@users.noreply.github.com>
Co-authored-by: Edu González de la Herrán <25320357+eedugon@users.noreply.github.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.
agree with edu about the bullets, but we can take that on at a later date. did a little content pass while I was here (sorry for the re-review)
deploy-manage/deploy/elastic-cloud/restrictions-known-problems.md
Outdated
Show resolved
Hide resolved
|
LGTM as well and agreed on the later cleanup, thanks so much @kunisen et al! ⭐ |
Co-authored-by: shainaraskas <58563081+shainaraskas@users.noreply.github.com>
Co-authored-by: shainaraskas <58563081+shainaraskas@users.noreply.github.com>
Co-authored-by: shainaraskas <58563081+shainaraskas@users.noreply.github.com>
Co-authored-by: shainaraskas <58563081+shainaraskas@users.noreply.github.com>
|
Thank you all for the kind help and valuable comments! 🙇 |
Description
This is to address an internal ticket - link
Preview before PR merge
Preview after PR merge
cc @maggieghamry @AlexP-Elastic @jakommo