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

Porchctl command fixes: #81

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

dimitar-axyomcore
Copy link

Fixes for porchctl repo commands described in nephio-project/nephio#755 issue.

  • repo get: Output n=Namespace column when -A/--all flag specified.
  • repo reg: Use kubeconfig context namespace if -n/--namespace flag not provided.
  • repo unreg: Use kubeconfig context namespace if -n/--namespace flag not provided.

I've resubmitted this PR under new GH account to be able to sign the corporate CDL.

I would need guidance on how to approach tests for the commands. I've noticed the repo reg command uses a fake porch server to handle the tests cases. Should I follow suit for commands affected by this PR?

- repo get: Output namespace column when -A/--all flag specified.
- repo reg: Use kubeconfig context namespace if -n/--namespace flag not
  provided.
- repo unreg: Use kubeconfig context namespace if -n/--namespace flag
  not provided.
@nephio-prow nephio-prow bot requested review from s3wong and tliron June 25, 2024 18:12
Copy link
Contributor

nephio-prow bot commented Jun 25, 2024

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: dimitar-axyomcore
Once this PR has been reviewed and has the lgtm label, please assign tliron for approval by writing /assign @tliron in a comment. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found 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

Copy link

CLA Missing ID CLA Not Signed

Copy link
Contributor

nephio-prow bot commented Jun 25, 2024

Hi @dimitar-axyomcore. Thanks for your PR.

I'm waiting for a nephio-project 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.

@dimitar-axyomcore
Copy link
Author

I am currently working on signing the Corporate CLA

@liamfallon
Copy link
Member

/ok-to-test

@dimitar-axyomcore
Copy link
Author

At this point I am not sure if the corporate Easy CLA will be signed. Please feel free to assign this issue to someone else.
I really appreciate you were waiting for that long :-)

@efiacor efiacor self-assigned this Aug 13, 2024
@dimitar-axyomcore
Copy link
Author

Hi @efiacor ,

I made one more effort to sign the corporate CLA. Please give me a few more days and hopefully I could start working on the PR next week.

@dgeorgievski
Copy link

Hi @efiacor ,

I made one more effort to sign the corporate CLA. Please give me a few more days and hopefully I could start working on the PR next week.

it looks like my company is not ready for contributing to Open Source projects, yet. Just to give some context, Axyom.Core was spun off Casa Systems last May and our current priorities don't have room for this task.

We have two options. Either I re-open the initial PR opened under my personal GitHub account or you guys complete it. I know the PR has been opened for quite some time now.

In any case, thanks for waiting on my response.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants