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

📖 add migration guide from v2 to v3 #1714

Merged
merged 1 commit into from
Oct 28, 2020
Merged

📖 add migration guide from v2 to v3 #1714

merged 1 commit into from
Oct 28, 2020

Conversation

camilamacedo86
Copy link
Member

@camilamacedo86 camilamacedo86 commented Oct 8, 2020 •
edited
Loading

Description

  • add migration guide from v2 to v3 with the alpha plugin steps and info as well.

Motivation

  • allow users to migrate their project to use the plugins (v3 project version)
  • allow users to try out and use the v3-alpha plugin as well
  • clarifies the significant changes in the scaffold projects with v3-alpha

Preview
https://deploy-preview-1714--kubebuilder.netlify.app/migration/v2vsv3.html

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 8, 2020
@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 8, 2020
@camilamacedo86 camilamacedo86 force-pushed the migration-guide-v2-v3 branch 2 times, most recently from 4cd65ff to 14f6b33 Compare October 8, 2020 12:53
@camilamacedo86 camilamacedo86 mentioned this pull request Oct 8, 2020
Closed
@camilamacedo86
Copy link
Member Author

c/c @gabbifish @estroz

@camilamacedo86
Copy link
Member Author

/test pull-kubebuilder-e2e-k8s-1-15-3

@camilamacedo86
Copy link
Member Author

/test pull-kubebuilder-e2e-k8s-1-15-3

NicolasT
NicolasT reviewed Oct 15, 2020
View reviewed changes
Copy link

@NicolasT NicolasT left a comment

Choose a reason for hiding this comment

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

Some edit suggestions to (IMHO) increase readability. Not a native speaker though so more eyeballs won't hurt.

docs/book/src/migration/migration_guide_v2tov3.md Outdated Show resolved Hide resolved
docs/book/src/migration/migration_guide_v2tov3.md Outdated Show resolved Hide resolved
docs/book/src/migration/migration_guide_v2tov3.md Outdated Show resolved Hide resolved
docs/book/src/migration/migration_guide_v2tov3.md Outdated Show resolved Hide resolved
docs/book/src/migration/migration_guide_v2tov3.md Outdated Show resolved Hide resolved
docs/book/src/migration/migration_guide_v2tov3.md Outdated Show resolved Hide resolved
docs/book/src/migration/migration_guide_v2tov3.md Outdated Show resolved Hide resolved
docs/book/src/migration/v2vsv3.md Outdated Show resolved Hide resolved
docs/book/src/migration/v2vsv3.md Outdated Show resolved Hide resolved
docs/book/src/migration/v2vsv3.md Outdated Show resolved Hide resolved
@camilamacedo86
Copy link
Member Author

Hi @NicolasT,

Really thank you for your help on it. All suggestions made in the review are addressed now. Please, feel free to re-check.

Adirio
Adirio reviewed Oct 27, 2020
View reviewed changes
Copy link
Contributor

@Adirio Adirio left a comment •
edited
Loading

Choose a reason for hiding this comment

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

You ahve very long lines, try using lines no longer than 80 characters so that it can be read better from mobile devices.

Several comments:

docs/book/src/SUMMARY.md Show resolved Hide resolved
docs/book/src/migration/v2vsv3.md Outdated Show resolved Hide resolved
docs/book/src/migration/migration_guide_v2tov3.md Outdated Show resolved Hide resolved
docs/book/src/migration/migration_guide_v2tov3.md Show resolved Hide resolved
docs/book/src/migration/migration_guide_v2tov3.md Outdated Show resolved Hide resolved
docs/book/src/migration/migration_guide_v2tov3.md Outdated Show resolved Hide resolved
docs/book/src/migration/migration_guide_v2tov3.md Outdated Show resolved Hide resolved
camilamacedo86
camilamacedo86 commented Oct 27, 2020
View reviewed changes
Copy link
Member Author

@camilamacedo86 camilamacedo86 left a comment

Choose a reason for hiding this comment

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

HI @Adirio,

Thank you for your review. All suggestions applied.

Adirio
Adirio reviewed Oct 27, 2020
View reviewed changes
docs/book/src/SUMMARY.md Outdated Show resolved Hide resolved
docs/book/src/migration/migration_guide_v2tov3.md Outdated Show resolved Hide resolved
@camilamacedo86 camilamacedo86 force-pushed the migration-guide-v2-v3 branch 2 times, most recently from b090c68 to a39bb79 Compare October 27, 2020 14:04
@Adirio
Copy link
Contributor

Adirio commented Oct 27, 2020 •
edited
Loading

LGTM, leaving someone more up-to-date with v3 to give the lgtm label as I haven't tested the steps given in this migration guide, just checked the content of the guide.

As a minor minor NIT I would split long lines into smaller one so that they are easier to read in small screens.

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Adirio, camilamacedo86

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

@camilamacedo86
Copy link
Member Author

/test pull-kubebuilder-e2e-k8s-1-14-1

1 similar comment
@camilamacedo86
Copy link
Member Author

/test pull-kubebuilder-e2e-k8s-1-14-1

pwittrock
pwittrock reviewed Oct 27, 2020
View reviewed changes
docs/book/src/migration/v2vsv3.md Outdated
@@ -0,0 +1,14 @@
# Kubebuilder v2 vs v3

This document cover all breaking changes when migrating from v2 to v3.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: document covers

Copy link
Member Author

Choose a reason for hiding this comment

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

done 👍

@pwittrock
Copy link
Contributor

LGTM

@Adirio
Copy link
Contributor

Adirio commented Oct 28, 2020

/test pull-kubebuilder-test

@pwittrock
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 28, 2020
@k8s-ci-robot k8s-ci-robot merged commit 67f91f7 into kubernetes-sigs:master Oct 28, 2020
@camilamacedo86 camilamacedo86 mentioned this pull request Nov 1, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@NicolasT NicolasT NicolasT left review comments

@pwittrock pwittrock pwittrock left review comments

@Adirio Adirio Adirio approved these changes

@droot droot Awaiting requested review from droot

@mengqiy mengqiy Awaiting requested review from mengqiy

Assignees

@pwittrock pwittrock

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", 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.
5 participants
@camilamacedo86 @Adirio @k8s-ci-robot @pwittrock @NicolasT