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

📖 update with latest changes and fix the samples project used in the docs #1850

Merged
merged 1 commit into from
Nov 30, 2020
Merged

📖 update with latest changes and fix the samples project used in the docs #1850

merged 1 commit into from
Nov 30, 2020

Conversation

camilamacedo86
Copy link
Member

@camilamacedo86 camilamacedo86 commented Nov 18, 2020 •
edited
Loading

Description

Update and fix the samples used in the docs by:

Motivation

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Nov 18, 2020
@camilamacedo86
Copy link
Member Author

/test pull-kubebuilder-e2e-k8s-1-18-0

@camilamacedo86
Copy link
Member Author

/test pull-kubebuilder-e2e-k8s-1-18-0

@camilamacedo86 camilamacedo86 changed the title 📖 update/fix project samples used in the docs 📖 update with latest changes and fix the samples project used in the docs Nov 18, 2020
christopherhein
christopherhein reviewed Nov 18, 2020
View reviewed changes
docs/book/src/cronjob-tutorial/cronjob-tutorial.md
@@ -46,7 +46,7 @@ project:
```bash
# we'll use a domain of tutorial.kubebuilder.io,
# so all API groups will be <group>.tutorial.kubebuilder.io.
kubebuilder init --domain tutorial.kubebuilder.io
kubebuilder init --plugins=go/v3-alpha --domain tutorial.kubebuilder.io
Copy link
Member

Choose a reason for hiding this comment

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

Assuming we'll update this post stability time?

Copy link
Member Author

@camilamacedo86 camilamacedo86 Nov 22, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Yes. My idea is to update that for go/v3 when its stable version will be released.

christopherhein
christopherhein reviewed Nov 18, 2020
View reviewed changes
...onjob-tutorial/testdata/project/config/crd/bases/batch.tutorial.kubebuilder.io_cronjobs.yaml
description: One and only one of the following
should be specified. Exec specifies the
action to take.

Copy link
Member

Choose a reason for hiding this comment

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

Just have to confirm, this is meant to be over 7k lines for this CRD?

Copy link
Member

Choose a reason for hiding this comment

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

Since the diff is so big it's hard to load in PR reviewing - https://github.com/kubernetes-sigs/kubebuilder/blob/8bd0cbfd38d7027b1bf60d3f156f0d4f00f654db/docs/book/src/cronjob-tutorial/testdata/project/config/crd/bases/batch.tutorial.kubebuilder.io_cronjobs.yaml for clarity since this comment shows that it's on line 1923.

Copy link
Member Author

@camilamacedo86 camilamacedo86 Nov 23, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

The CRD are generated via make manifests so it shows fine.
To ensure that I re-generated the project locally with the helpers (generate_multiversion.sh and generate_cronjob.sh) scripts and executed make manifests before add the docs comments.

christopherhein
christopherhein approved these changes Nov 30, 2020
View reviewed changes
Copy link
Member

@christopherhein christopherhein left a comment

Choose a reason for hiding this comment

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

/lgtm

Nice work @camilamacedo86 !

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 30, 2020
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

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

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

@k8s-ci-robot k8s-ci-robot merged commit f27d2f7 into kubernetes-sigs:master Nov 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@christopherhein christopherhein christopherhein approved these changes

@estroz estroz Awaiting requested review from estroz

@mengqiy mengqiy Awaiting requested review from mengqiy

@gabbifish gabbifish Awaiting requested review from gabbifish

Assignees

@christopherhein christopherhein

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/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

The CustomResourceDefinition "cronjobs.batch.tutorial.kubebuilder.io" is invalid
3 participants
@camilamacedo86 @k8s-ci-robot @christopherhein