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

Add link to the reset offset learning resource page #1048

Closed
wants to merge 2 commits into from
Closed
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
StackItem,
Text,
} from "@/libs/patternfly/react-core";
import { isProductizedBuild } from "@/utils/env";
import { useTranslations } from "next-intl";
import { useRouter } from "next/navigation";

Expand Down Expand Up @@ -69,11 +70,13 @@ export function ResetOffsetModal({
<StackItem>
<Text>{t("shutdown_active_members")}</Text>
</StackItem>
<StackItem>
<ExternalLink testId={"learn_to_shutdown_members"} href={""}>
{t("learn_to_shutdown_members")}
</ExternalLink>
</StackItem>
{isProductizedBuild && (
<StackItem>
<ExternalLink testId={"learn_to_shutdown_members"} href={"https://docs.redhat.com/en/documentation/red_hat_streams_for_apache_kafka/2.7/html-single/kafka_configuration_tuning/index#managing_offset_policy"}>
{t("learn_to_shutdown_members")}
</ExternalLink>
Copy link
Member

Choose a reason for hiding this comment

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

The label and link should maybe go under learning.labels and learning.links. We should also have a link to something in the Apache Kafka documentation for upstream.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@MikeEdgar Is the phrase "Learn how to shut down members" correct to include under learning resources?

Copy link
Member

Choose a reason for hiding this comment

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

@hemahg , yes that seems good.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Isn't the wording a bit... aggressive? Not a native speaker here, but it feels that way

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, perhaps something like "Learn how to stop consumers" would be better? I'm not sure there is documentation specific to that since it is very dependent on how the consumers are coded. I don't think the link about managing offset policy matches the label.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@shirimordechay, We've been discussing the phrasing for the learning resource, and I wanted to get some additional feedback. @MikeEdgar suggested "Learn how to stop consumers" as a more appropriate phrase, but I'd like to make sure everyone is on board with this change. Do you have any thoughts or suggestions for improving the wording or can help find relevant documentation links?

Copy link

@shirimordechay shirimordechay Sep 17, 2024

Choose a reason for hiding this comment

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

Rephrasing sounds good to me. I believe the doc link is the one @[email protected] has provided, but I'm assuming we'll have a new link for GA. need to confirm that with Paul.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Update the phrase as per @MikeEdgar suggestion, there is no specific document to shutdown the consumer group members

</StackItem>
)}
</Stack>
</Modal>
);
Expand Down