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.

Sign up for GitHub

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

Jump to bottom

Docs: Improving the install with keadm documentation #544

Merged
merged 7 commits into from
Jun 1, 2024
Merged

Docs: Improving the install with keadm documentation #544

merged 7 commits into from
Jun 1, 2024

Conversation

1Shubham7
Copy link
Contributor

@1Shubham7 1Shubham7 commented May 17, 2024 •
edited

  • Please check if the PR fulfills these requirements
  • The commit message follows our guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
    docs update

this is one among series of PRs planned to improve and rewrite some parts of documentation.

Works towards solving #533

@1Shubham7
Part 1: improving the install with keadm docs
e5e989d 
Signed-off-by: Shubham Singh <shubhammahar1306@gmail.com>
@kubeedge-bot kubeedge-bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label May 17, 2024
@1Shubham7
Copy link
Contributor Author

@fisherxu @wbc6080, I need to know whether we should be using "Keadm" or "keadm" consistently in the docs.

@1Shubham7
Part 2: improving the install with keadm docs
466e931 
Signed-off-by: Shubham Singh <shubhammahar1306@gmail.com>
@kubeedge-bot kubeedge-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 18, 2024
fujitatomoya
fujitatomoya reviewed May 21, 2024
View reviewed changes
Copy link
Contributor

@fujitatomoya fujitatomoya left a comment

Choose a reason for hiding this comment

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

overall, ltgm. maybe we can ask for maintainers about names of terminologies.

docs/setup/install-with-keadm.md Outdated Show resolved Hide resolved
docs/setup/install-with-keadm.md Outdated Show resolved Hide resolved
docs/setup/install-with-keadm.md Show resolved Hide resolved
docs/setup/install-with-keadm.md Outdated Show resolved Hide resolved
wbc6080
wbc6080 reviewed May 22, 2024
View reviewed changes
docs/setup/install-with-keadm.md Outdated Show resolved Hide resolved
docs/setup/install-with-keadm.md Show resolved Hide resolved
docs/setup/install-with-keadm.md Outdated Show resolved Hide resolved
@1Shubham7
fixed suggestions from reviewers
bbb1b2f 
Signed-off-by: GitHub <noreply@github.com>
@1Shubham7
fixed suggestions from reviewers (removing extra files)
5b86717 
Signed-off-by: GitHub <noreply@github.com>
@1Shubham7
edgecode -> EdgeCore
1351443 
Signed-off-by: GitHub <noreply@github.com>
@1Shubham7
small fix
d5be0c6 
Signed-off-by: GitHub <noreply@github.com>
fujitatomoya
fujitatomoya approved these changes May 22, 2024
View reviewed changes
Copy link
Contributor

@fujitatomoya fujitatomoya left a comment

Choose a reason for hiding this comment

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

lgtm

@kubeedge-bot kubeedge-bot added the lgtm Indicates that a PR is ready to be merged. label May 22, 2024
wbc6080
wbc6080 reviewed May 25, 2024
View reviewed changes
Copy link
Contributor

@wbc6080 wbc6080 left a comment

Choose a reason for hiding this comment

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

/lgtm
Thanks for your contribution
cc @Shelley-BaoYue @fisherxu

Shelley-BaoYue
Shelley-BaoYue reviewed May 25, 2024
View reviewed changes
docs/setup/install-with-keadm.md Outdated Show resolved Hide resolved
docs/setup/install-with-keadm.md Outdated Show resolved Hide resolved
docs/setup/install-with-keadm.md Outdated Show resolved Hide resolved
docs/setup/install-with-keadm.md Outdated Show resolved Hide resolved
@kubeedge-bot kubeedge-bot added do-not-merge/invalid-commit-message Indicates that a PR should not merge because it has an invalid commit message. and removed lgtm Indicates that a PR is ready to be merged. labels May 25, 2024
@kubeedge-bot kubeedge-bot removed the do-not-merge/invalid-commit-message Indicates that a PR should not merge because it has an invalid commit message. label May 25, 2024
@1Shubham7
adding suggestions from Shelley-BaoYue
4368428 
Signed-off-by: GitHub <noreply@github.com>
Shelley-BaoYue
Shelley-BaoYue reviewed May 27, 2024
View reviewed changes
Copy link
Collaborator

@Shelley-BaoYue Shelley-BaoYue left a comment

Choose a reason for hiding this comment

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

/lgtm

@kubeedge-bot kubeedge-bot added the lgtm Indicates that a PR is ready to be merged. label May 27, 2024
@1Shubham7
Copy link
Contributor Author

@wbc6080 @fujitatomoya there have been some changes after you approved. Please approve this again.

fujitatomoya
fujitatomoya approved these changes May 29, 2024
View reviewed changes
Copy link
Contributor

@fujitatomoya fujitatomoya left a comment

Choose a reason for hiding this comment

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

thanks for iterating and being responsive. looks good to me.

@fujitatomoya
Copy link
Contributor

i will leave this to @Shelley-BaoYue @wbc6080

wbc6080
wbc6080 reviewed May 30, 2024
View reviewed changes
Copy link
Contributor

@wbc6080 wbc6080 left a comment

Choose a reason for hiding this comment

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

/lgtm

@1Shubham7 1Shubham7 mentioned this pull request May 30, 2024
Open
@1Shubham7
Copy link
Contributor Author

@wbc6080 , @Shelley-BaoYue please approve this so that the PR gets merged. Also checkout the other improvement PRs that I have made :)

Shelley-BaoYue
Shelley-BaoYue reviewed Jun 1, 2024
View reviewed changes
Copy link
Collaborator

@Shelley-BaoYue Shelley-BaoYue left a comment

Choose a reason for hiding this comment

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

/approve

@kubeedge-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fujitatomoya, Shelley-BaoYue

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

@kubeedge-bot kubeedge-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 1, 2024
@kubeedge-bot kubeedge-bot merged commit 54b0a52 into kubeedge:master Jun 1, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wbc6080 wbc6080 wbc6080 left review comments

@Shelley-BaoYue Shelley-BaoYue Shelley-BaoYue left review comments

@fujitatomoya fujitatomoya fujitatomoya approved these changes

@anvithks anvithks Awaiting requested review from anvithks

Assignees

@fujitatomoya fujitatomoya

@wbc6080 wbc6080

@Shelley-BaoYue Shelley-BaoYue

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. 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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@1Shubham7 @fujitatomoya @kubeedge-bot @wbc6080 @Shelley-BaoYue