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 Karmada x PipeCD guideline #680

Merged
merged 1 commit into from
Sep 12, 2024

Conversation

khanhtc1202
Copy link
Contributor

What type of PR is this?

/kind documentation

What this PR does / why we need it:

This document explains how to use Karmada x PipeCD as a way to deploy the application's manifest resource to multi-cluster. Follow what we did at KubeCon HongKong 2024 (ref: https://sched.co/1eYYs)

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

@karmada-bot karmada-bot added the kind/documentation Categorizes issue or PR as related to documentation. label Sep 5, 2024
@karmada-bot
Copy link
Collaborator

Welcome @khanhtc1202! It looks like this is your first PR to karmada-io/website 🎉

@karmada-bot karmada-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Sep 5, 2024
@RainbowMango
Copy link
Member

Hi @khanhtc1202 You might need to update the side bars as well, otherwise, I can't get the preview.

@khanhtc1202
Copy link
Contributor Author

@RainbowMango Thanks for pointing out, I updated the PR 😄

Copy link
Member

@RainbowMango RainbowMango left a comment

Choose a reason for hiding this comment

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

/assign
Thanks, I will take a look ASAP.

Comment on lines +45 to +46
pipedID: <YOUR_PIPED_ID>
pipedKeyData: <YOUR_PIPED_KEY_DATA>
Copy link
Member

Choose a reason for hiding this comment

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

Do you want to explain how to set the YOUR_PIPED_ID and YOUR_PIPED_KEY_DATA a little bit?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I referenced the quickstart docs for pipecd installation in the previous section already, so I think users are already know with it us up to this place. wdyt? 🤔

Copy link
Member

Choose a reason for hiding this comment

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

Why not just update it to test as you give an example with that ID.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since that place requires not the piped name but the piped ID, which is different depending on users in the registered time, I think it could be less confusing if we keep it as is instead of mentioning the test piped name here. Users would also know that they need to find their <YOUR_PIPED_ID> and <YOUR_PIPED_KEY_DATA>, which could be found if they follow the quickstart docs.

Copy link
Contributor Author

@khanhtc1202 khanhtc1202 Sep 11, 2024

Choose a reason for hiding this comment

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

Also, if you still think it could be less confusing with the explanation, I can add something like

For <YOUR_PIPED_ID> and <YOUR_PIPED_KEY_DATA>, please refer to [installing piped](https://pipecd.dev/docs-v0.48.x/quickstart/#12-installing-piped) docs.

WDYT? 🤔

Copy link
Member

Choose a reason for hiding this comment

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

Much better now. Thanks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed by 0fdbb78 🙏

@RainbowMango
Copy link
Member

OK. Last question, do you want to squash your commits?

@khanhtc1202
Copy link
Contributor Author

@RainbowMango Oh, I didn't notice that I usually use the merge and squash feature on GitHub UI. Let me try squash in my local 👍

@karmada-bot karmada-bot added the do-not-merge/contains-merge-commits Indicates a PR which contains merge commits. label Sep 12, 2024
@karmada-bot karmada-bot removed the do-not-merge/contains-merge-commits Indicates a PR which contains merge commits. label Sep 12, 2024
Signed-off-by: khanhtc1202 <[email protected]>

Add link to sidebar

Signed-off-by: khanhtc1202 <[email protected]>

Add explanation about piped id and key

Signed-off-by: khanhtc1202 <[email protected]>
@khanhtc1202
Copy link
Contributor Author

@RainbowMango I squashed all commits, please check when you have time. Thank you 😄

Copy link
Member

@RainbowMango RainbowMango left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@karmada-bot karmada-bot added the lgtm Indicates that a PR is ready to be merged. label Sep 12, 2024
@karmada-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: RainbowMango

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@karmada-bot karmada-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 12, 2024
@karmada-bot karmada-bot merged commit 260a6f4 into karmada-io:main Sep 12, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/documentation Categorizes issue or PR as related to documentation. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants