Skip to content
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

Improvements to UCN dynamic client docs #1086

Merged

Conversation

JamesHazelcast
Copy link
Contributor

@JamesHazelcast JamesHazelcast commented Apr 17, 2024

Following discussion on Slack, we identified some improvements that can be made to this page to clarify functionality to users.

More specifically I have split the page into 2 parts, 1 focusing on creating a new UCN from clients, and 1 focusing on updating an existing UCN. I've also adjusted some wording to make this difference clearer.

Relevant slack thread: https://hazelcast.slack.com/archives/C035HQET5/p1713272907972899

Following discussion on Slack, we identified some improvements that can be made to this page to clarify functionality to users.

Relevant slack thread: https://hazelcast.slack.com/archives/C035HQET5/p1713272907972899
@JamesHazelcast JamesHazelcast added the documentation Improvements or additions to documentation label Apr 17, 2024
@JamesHazelcast JamesHazelcast added this to the 5.5 milestone Apr 17, 2024
@JamesHazelcast JamesHazelcast self-assigned this Apr 17, 2024
Copy link

netlify bot commented Apr 17, 2024

Deploy Preview for hardcore-allen-f5257d ready!

Name Link
🔨 Latest commit 6cca0fe
🔍 Latest deploy log https://app.netlify.com/sites/hardcore-allen-f5257d/deploys/66261b03151a32000818c1d7
😎 Deploy Preview https://deploy-preview-1086--hardcore-allen-f5257d.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

docs/modules/clusters/pages/ucn-dynamic-client.adoc Outdated Show resolved Hide resolved
docs/modules/clusters/pages/ucn-dynamic-client.adoc Outdated Show resolved Hide resolved
docs/modules/clusters/pages/ucn-dynamic-client.adoc Outdated Show resolved Hide resolved
docs/modules/clusters/pages/ucn-dynamic-client.adoc Outdated Show resolved Hide resolved
docs/modules/clusters/pages/ucn-dynamic-client.adoc Outdated Show resolved Hide resolved
docs/modules/clusters/pages/ucn-dynamic-client.adoc Outdated Show resolved Hide resolved
Co-authored-by: rebekah-lawrence <142301480+rebekah-lawrence@users.noreply.github.com>
Copy link
Contributor

@rebekah-lawrence rebekah-lawrence left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Nice to know you enjoy my reviews; over the years they have been called many things (pedantic, picky, a pain in the ***), but not often enjoyable ;-)

And, yes, I edited my own original text - reading it with fresh eyes, the note interrupted my flow; this one was to tie the paragraphs together - and probably pedantic!

Great work and thanks for picking it up.

Copy link
Contributor

@Serdaro Serdaro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with a minor comment.

docs/modules/clusters/pages/ucn-dynamic-client.adoc Outdated Show resolved Hide resolved
JamesHazelcast and others added 2 commits April 19, 2024 13:24
Co-authored-by: Serdar Ozmen <serdar@hazelcast.com>
@JamesHazelcast
Copy link
Contributor Author

JamesHazelcast commented May 28, 2024

Hi @oliverhowell @rebekah-lawrence do you know what the situation is with this PR? Is it intentionally left open? In the past I have left PR merging to docs team, but I see other 5.5 PRs have been merged recently, so wanted to check in about this one.

@rebekah-lawrence
Copy link
Contributor

Hey @JamesHazelcast I generally leave the originator to merge the PR once they have all reviews/approvals required.

Only question on this one is whether it needs to be backported to 5.4. Let me know and I'm happy to merge or add the label and merge.

@oliverhowell I have never merged someone else's PR, but this might be something for consideration in our processes so it is clear to all parties what the expected flow of a PR is to be as we get all our stuff properly documented.

@JamesHazelcast
Copy link
Contributor Author

Thanks for the info @rebekah-lawrence, I'm happy to merge myself, just didn't want to impact any process that may have been in place.

I think this is worth backporting to 5.4, so I've added the label and will merge this PR 👍

@JamesHazelcast JamesHazelcast merged commit c464fff into hazelcast:main May 29, 2024
6 checks passed
@JamesHazelcast JamesHazelcast deleted the update/5.5/ucn-dynamic-client branch May 29, 2024 10:39
github-actions bot pushed a commit that referenced this pull request May 29, 2024
Improvements to UCN dynamic client docs

Following discussion on Slack, we identified some improvements that can be made to this page to clarify functionality to users.

Relevant slack thread: https://hazelcast.slack.com/archives/C035HQET5/p1713272907972899

(cherry picked from commit c464fff)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport to 5.4 documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants