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.

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

Add kubectl trace v0.1.0-rc.1 #658

Merged
merged 3 commits into from
Jun 30, 2020

Conversation

fntlnz
Copy link
Contributor

@fntlnz fntlnz commented Jun 19, 2020

Happy to submit kubectl trace to the krew index as per the discussions happening one year ago in iovisor/kubectl-trace#17

Sorry if it took a long time!

Adding a wip since I didn't test the manifest on all platforms yet.

If anyone can test it on windows before I do and report here I'll be grateful.

Signed-off-by: Lorenzo Fontana [email protected]

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jun 19, 2020
@k8s-ci-robot
Copy link
Contributor

Welcome @fntlnz!

It looks like this is your first PR to kubernetes-sigs/krew-index 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/krew-index has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

@k8s-ci-robot k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jun 19, 2020
plugins/trace.yaml Outdated Show resolved Hide resolved
plugins/trace.yaml Outdated Show resolved Hide resolved
@ahmetb
Copy link
Member

ahmetb commented Jun 19, 2020

If anyone can test it on windows before I do and report here I'll be grateful.

I suspect neither of the maintainers have a windows box. :(

Copy link
Contributor

@corneliusweig corneliusweig left a comment

Choose a reason for hiding this comment

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

As long as you know that your plugin runs on windows, it should be fine to also distribute the windows plugin. Binaries (.exe) usually work well---if you ignore the general hazzle of symlinks on windows. So I'm not too worried about this.

Thanks for contributing this. Very exciting to have it in krew-index!

plugins/trace.yaml Show resolved Hide resolved
Signed-off-by: Lorenzo Fontana <[email protected]>
@fntlnz fntlnz changed the title wip: Add kubectl trace v0.1.0-rc.1 Add kubectl trace v0.1.0-rc.1 Jun 22, 2020
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 22, 2020
@fntlnz
Copy link
Contributor Author

fntlnz commented Jun 22, 2020

@ahmetb @corneliusweig I just addressed all the suggestions and tested again.

Thank you two for the reviews, I really appreciate it. And thanks for this awesome tool!

@fntlnz
Copy link
Contributor Author

fntlnz commented Jun 22, 2020

Created the corresponding PR on kubectl trace for when this is merged. iovisor/kubectl-trace#118

Copy link
Contributor

@corneliusweig corneliusweig left a comment

Choose a reason for hiding this comment

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

So cool to finally have this in krew-index! For best impact, I'd advise to improve the description a bit.

plugins/trace.yaml Outdated Show resolved Hide resolved
plugins/trace.yaml Show resolved Hide resolved
plugins/trace.yaml Outdated Show resolved Hide resolved
plugins/trace.yaml Show resolved Hide resolved
Signed-Off-By: Lorenzo Fontana <[email protected]>

Co-authored-by: Cornelius Weig <[email protected]>
@fntlnz
Copy link
Contributor Author

fntlnz commented Jun 24, 2020

Thanks for your suggestions @corneliusweig - updated!

@ahmetb
Copy link
Member

ahmetb commented Jun 24, 2020

It seems description now only lists examples. Was that your intention @fntlnz? If so, I'll go ahead and merge. BTW you can set up an easy release automation to your repo so you don't have to come here to edit your yaml file every time you have a new version.

@fntlnz
Copy link
Contributor Author

fntlnz commented Jun 29, 2020

@ahmetb you're right the description is not what I wanted - updated it!

Wow didn't know this could be automated! Good work, thanks @ahmetb !

I opened an issue to do that iovisor/kubectl-trace#119

@ahmetb
Copy link
Member

ahmetb commented Jun 30, 2020

/lgtm
/approve

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

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ahmetb, fntlnz

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 added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 30, 2020
@k8s-ci-robot k8s-ci-robot merged commit ec7bee6 into kubernetes-sigs:master Jun 30, 2020
@fntlnz fntlnz deleted the kubectl-trace branch July 2, 2020 08:48
@fntlnz
Copy link
Contributor Author

fntlnz commented Jul 2, 2020

Thanks everyone!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants