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

Porchctl repo command fixes #80

Merged
merged 2 commits into from
Nov 14, 2024
Merged

Porchctl repo command fixes #80

merged 2 commits into from
Nov 14, 2024

Conversation

dgeorgievski
Copy link
Contributor

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

I wasn't sure what approach to take for testing the repo commands. I see that repo reg command is using fake porch server. Should I follow suit for repo reg command?

@nephio-prow nephio-prow bot requested a review from henderiw June 24, 2024 16:22
@linux-foundation-easycla Linux Foundation: EasyCLA
Copy link

linux-foundation-easycla bot commented Jun 24, 2024 •
edited
Loading

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: dgeorgievski / name: Dimitar Georgievski (1152765, b6a401e)

@nephio-prow nephio-prow bot requested a review from johnbelamaric June 24, 2024 16:22
@nephio-prow Nephio PROW
Copy link
Contributor

nephio-prow bot commented Jun 24, 2024

Hi @dgeorgievski. 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.

@dgeorgievski
Copy link
Contributor Author

dgeorgievski commented Jun 24, 2024 •
edited
Loading

Question about the CLA agreement. Axyom.core , my employer, is interested to sign the corporate CLA. I have a question for the email and GitHub account used in this PR.

So far, I've been working with my personal GitHub account and email address. Would I need to re-submit the PR with the corporate email and GitHub account linked to it?

@dgeorgievski
Copy link
Contributor Author

I am closing this PR and will open a new one with Axyom.Core credentials to be able to submit Corporate CLA request.

@dgeorgievski dgeorgievski mentioned this pull request Nov 11, 2024
Closed
@liamfallon liamfallon reopened this Nov 12, 2024
@liamfallon
Copy link
Member

/ok-to-test

@dgeorgievski
Copy link
Contributor Author

Easy CLA signed

@liamfallon
Copy link
Member

Easy CLA signed

It seems there is still some problem with the CLA, the check is not passing.Maybe submit a support request on the link above.

@dgeorgievski
Copy link
Contributor Author

Easy CLA signed

It seems there is still some problem with the CLA, the check is not passing.Maybe submit a support request on the link above.

I have received a copy of the signed agreement, so I am not sure why the check is failing. I will contact support.

@dgeorgievski
1. porchctl repo add - add namespace columng when -A flag specified
b6a401e 
2. porchctl repo reg - use current kubeconfig context namespace if it is
   not provided.
3. Use kubeconfig context namespace if flag not provided
4. fixed author's email address to comply with EasyCLA
@dgeorgievski
Copy link
Contributor Author

Easy CLA signed

It seems there is still some problem with the CLA, the check is not passing.Maybe submit a support request on the link above.

I have received a copy of the signed agreement, so I am not sure why the check is failing. I will contact support.

EasyCLA fixed. Proceeding with the next steps. Again, thank you for patience.

@liamfallon
Copy link
Member

/retest

liamfallon
liamfallon reviewed Nov 14, 2024
View reviewed changes
Copy link
Member

@liamfallon liamfallon left a comment

Choose a reason for hiding this comment

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

@dgeorgievski thanks for the contribution!

/approve
/lgtm

@nephio-prow nephio-prow bot added the lgtm label Nov 14, 2024
@nephio-prow Nephio PROW
Copy link
Contributor

nephio-prow bot commented Nov 14, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dgeorgievski, liamfallon

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

@nephio-prow nephio-prow bot added the approved label Nov 14, 2024
@nephio-prow nephio-prow bot merged commit a974d65 into nephio-project:main Nov 14, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@liamfallon liamfallon liamfallon left review comments

@henderiw henderiw Awaiting requested review from henderiw

@johnbelamaric johnbelamaric Awaiting requested review from johnbelamaric

Assignees

@liamfallon liamfallon

Labels
approved lgtm ok-to-test
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@dgeorgievski @liamfallon