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

Fix typos in DEVELOPMENT.md file #7179

Closed
wants to merge 1 commit into from
Closed

Fix typos in DEVELOPMENT.md file #7179

wants to merge 1 commit into from

Conversation

shankarpentyala07
Copy link
Contributor

@shankarpentyala07 shankarpentyala07 commented Oct 5, 2023 •
edited by pritidesai
Loading

Changes

  1. In DEVELOPMENT.md file , to configure upstream optional , while set up a fork , the instructions throws an error when used in zsh shell , because of # symbol not found.
  2. In setup cluster of type kind , the word kind is in Camel case Kind , changed it to lower case kind
  3. The update or add new crd types in Iterating on code changes section, doesn't navigate and has been corrected.

Submitter Checklist

As the author of this PR, please check off the items in this checklist:

  • Has Docs if any changes are user facing, including updates to minimum requirements e.g. Kubernetes version bumps
  • Has Tests included if any functionality added or changed
  • Follows the commit message standard
  • Meets the Tekton contributor standards (including functionality, content, code)
  • Has a kind label. You can add one by adding a comment on this PR that contains /kind <type>. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tep
  • Release notes block below has been updated with any user facing changes (API changes, bug fixes, changes requiring upgrade notices or deprecation warnings). See some examples of good release notes.
  • Release notes contains the string "action required" if the change requires additional action from users switching to the new release

Release Notes

NONE

@tekton-robot tekton-robot added the release-note-none Denotes a PR that doesnt merit a release note. label Oct 5, 2023
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Oct 5, 2023 •
edited
Loading

CLA Not Signed

@tekton-robot tekton-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Oct 5, 2023
@tekton-robot
Copy link
Collaborator

Hi @shankarpentyala07. Thanks for your PR.

I'm waiting for a tektoncd member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@shankarpentyala07 shankarpentyala07 changed the title Issue 7013 Fix typos in DEVELOPMENT.md file Oct 5, 2023
@shankarpentyala07
Copy link
Contributor Author

/kind documentation

@tekton-robot tekton-robot added the kind/documentation Categorizes issue or PR as related to documentation. label Oct 5, 2023
@Yongxuanzhang
Copy link
Member

/ok-to-test

@tekton-robot tekton-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Oct 6, 2023
Yongxuanzhang
Yongxuanzhang reviewed Oct 6, 2023
View reviewed changes
Copy link
Member

@Yongxuanzhang Yongxuanzhang left a comment

Choose a reason for hiding this comment

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

could you squash your commits into 1? 🙏

DEVELOPMENT.md
Comment on lines 166 to +170
git remote add upstream git@github.com:tektoncd/pipeline.git
```

# Optional: Prevent accidental pushing of commits by changing the upstream URL to `no_push`
3. Optional: Prevent accidental pushing of commits by changing the upstream URL to `no_push`
```shell
Copy link
Member

Choose a reason for hiding this comment

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

Why need this change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for review
Prior , it is as below:
Screenshot 2023-10-05 at 11 23 52 PM

When copied and pasted in zsh shell , below error is thrown.

# Optional: Prevent accidental pushing of commits by changing the upstream URL to `no_push`
git remote set-url --push upstream no_push
zsh: command not found: no_push
zsh: command not found: #

When optional section is moved to new line as below, the issue is not seen
Screenshot 2023-10-05 at 11 26 08 PM

DEVELOPMENT.md
@@ -271,9 +273,9 @@ The recommended minimum development configuration is:
- 8 GB of (actual or virtualized) platform memory
- Node autoscaling, up to 3 nodes

#### Using [KinD](https://kind.sigs.k8s.io/)
Copy link
Member

Choose a reason for hiding this comment

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

I remember this was because it stands for K8s in Docker, but I'm ok with either

DEVELOPMENT.md
@@ -353,7 +355,7 @@ While iterating on code changes to the project, you may need to:
- Update your (external) dependencies with: `./hack/update-deps.sh`
- Update your type definitions with: `./hack/update-codegen.sh`
- Update your OpenAPI specs with: `./hack/update-openapigen.sh`
1. Update or [add new CRD types](#adding-new-types) as needed
1. Update or [add new CRD types](#adding-new-crd-types) as needed
Copy link
Member

Choose a reason for hiding this comment

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

👍

@shankarpentyala07
Copy link
Contributor Author

could you squash your commits into 1? 🙏

@Yongxuanzhang I have squashed the commits , Could you please review.

@Yongxuanzhang
Copy link
Member

Yongxuanzhang commented Oct 12, 2023 •
edited
Loading

could you squash your commits into 1? 🙏

@Yongxuanzhang I have squashed the commits , Could you please review.

Could you sign the CLA? I remember there is a button there and you need to click it?

@spzala
Copy link

spzala commented Oct 12, 2023

CLA approval in progress. Thanks!

@shankar-pentyala
Fix navigation to add new crd types in development.md
e3d1397 
In optional configuring upstream to no_push in Setup a fork section ,the content was modified to work in zsh shell
In Setup a Kubernetes cluster using kind , it was mentioned as Kind in camelcase , changed it to lower case
@pritidesai
Copy link
Member

Closing and reopening the PR to test the CLA 🙃

@pritidesai pritidesai closed this Oct 13, 2023
@pritidesai pritidesai reopened this Oct 13, 2023
pritidesai
pritidesai approved these changes Oct 13, 2023
View reviewed changes
Copy link
Member

@pritidesai pritidesai left a comment

Choose a reason for hiding this comment

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

Welcome to the Tekton Community @shankarpentyala07! Thanks for all the changes 👍

@tekton-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: pritidesai

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

@tekton-robot tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 13, 2023
@shankarpentyala07
Copy link
Contributor Author

Welcome to the Tekton Community @shankarpentyala07! Thanks for all the changes 👍

Thanks Preeti for the welcome & all your help,I am hoping to be an active contributor to the project.

@Yongxuanzhang
Copy link
Member

/lgtm

@tekton-robot tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 13, 2023
@pritidesai
Copy link
Member

Closing and reopening the PR to trigger EasyCLA!

@pritidesai pritidesai closed this Oct 16, 2023
@pritidesai pritidesai reopened this Oct 16, 2023
@Yongxuanzhang
Copy link
Member

@shankarpentyala07 could you double check if you have signed the CLA? Thanks!

@shankarpentyala07
Copy link
Contributor Author

Screenshot 2023-10-18 at 10 59 56 AM

Thanks @Yongxuanzhang .yes - I did try few times.

@Yongxuanzhang
Copy link
Member

Screenshot 2023-10-18 at 10 59 56 AM Thanks @Yongxuanzhang .yes - I did try few times.

Are you using different accounts to commit and open the PR?
image

@shankarpentyala07
Copy link
Contributor Author

Thanks @Yongxuanzhang for identifying the issue.
Found that, My local gitconfig has IBM org email and when opening the pull request , I was opening from my public Github.Both email id has been approved in the CLA list.

I have opened a new pull request with same changes and CLA passed : #7233. Shall I go ahead and close this pull request and use the new pull request for the changes.

@Yongxuanzhang
Copy link
Member

Thanks @Yongxuanzhang for identifying the issue. Found that, My local gitconfig has IBM org email and when opening the pull request , I was opening from my public Github.Both email id has been approved in the CLA list.

I have opened a new pull request with same changes and CLA passed : #7233. Shall I go ahead and close this pull request and use the new pull request for the changes.

Yeah, that's fine

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Yongxuanzhang Yongxuanzhang Yongxuanzhang left review comments

@pritidesai pritidesai pritidesai approved these changes

@afrittoli afrittoli Awaiting requested review from afrittoli

Assignees

@Yongxuanzhang Yongxuanzhang

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. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. release-note-none Denotes a PR that doesnt merit a release note. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@shankarpentyala07 @tekton-robot @Yongxuanzhang @spzala @pritidesai @shankar-pentyala