Skip to content

Conversation

Neutrollized
Copy link
Contributor

@Neutrollized Neutrollized commented Nov 23, 2023

Please ensure your pull request adheres to the following guidelines:

  • For first time contributors, read Submitting a pull request
  • All code is covered by unit and/or runtime tests where feasible.
  • All commits contain a well written commit description including a title,
    description and a Fixes: #XXX line if the commit addresses a particular
    GitHub issue.
  • If your commit description contains a Fixes: <commit-id> tag, then
    please add the commit author[s] as reviewer[s] to this issue.
  • All commits are signed off. See the section Developer’s Certificate of Origin
  • Provide a title or release-note blurb suitable for the release notes.
  • Are you a user of Cilium? Please add yourself to the Users doc
  • Thanks for contributing!

Using AKS-to-AKS Clustermesh Preparation guide as a template, I create one for GKE-to-GKE.

Also fixed a minor typo in the AKS guide

This is strictly a website/documentation addition. No changes to any Cilium code or functionality.

Signed-off-by: Neutrollized <glen.yu@gmail.com>
@Neutrollized Neutrollized requested review from a team as code owners November 23, 2023 04:24
@Neutrollized Neutrollized requested review from a user and YutaroHayakawa November 23, 2023 04:24
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Nov 23, 2023
@github-actions github-actions bot added the kind/community-contribution This was a contribution made by a community member. label Nov 23, 2023
Signed-off-by: Glen Yu <glen.yu@gmail.com>
@maintainer-s-little-helper
Copy link

Commit ca44122 does not match "(?m)^Signed-off-by:".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Nov 23, 2023
@Neutrollized
Copy link
Contributor Author

Neutrollized commented Nov 23, 2023

Realized my GitHub username was my user.name and went to amend the commit sign-off in command line but I think I messed it up and it wasn't included in the merge :(

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Hey @Neutrollized thanks for the PR.

It looks like your commits are signed-off, but the branch contains unnecessary commits - cb8c1fc is the same as the previous commit and then there is an unnecessary merge commit. Could you remove these two and force push your branch?

I left some comment about the docs structure. As you're following the AKS template, I don't consider them super blocking, but I think we could make the docs more clear for the user by reducing the duplication.

@ghost ghost added the release-note/misc This PR makes changes that have no direct user impact. label Nov 23, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot removed dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. labels Nov 23, 2023
Signed-off-by: Glen Yu <glen.yu@gmail.com>
@Neutrollized Neutrollized requested a review from a user November 23, 2023 19:17
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Thank you for the updates @Neutrollized! Looks good to me now, I just left comments with some minor things.

Signed-off-by: Glen Yu <glen.yu@gmail.com>
@Neutrollized Neutrollized requested a review from a user November 24, 2023 00:56
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

looks good to me, thanks

@ghost
Copy link

ghost commented Nov 24, 2023

/test

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Nov 27, 2023
@aanm aanm added this pull request to the merge queue Nov 27, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 27, 2023
@aanm aanm added this pull request to the merge queue Nov 27, 2023
Merged via the queue into cilium:main with commit ece4153 Nov 27, 2023
@Neutrollized Neutrollized deleted the gke-clustermesh-guide branch November 27, 2023 15:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/community-contribution This was a contribution made by a community member. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants