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

fixed path issue in import kvm, #303 #304 #305

Merged
merged 4 commits into from
Oct 5, 2023
Merged

Conversation

aliasgar55
Copy link
Contributor

Fixed issue with import kvm

@google-cla
Copy link

google-cla bot commented Sep 27, 2023

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

cmd/kvm/impkvms.go Outdated Show resolved Hide resolved
cmd/kvm/impkvms.go Outdated Show resolved Hide resolved
cmd/kvm/impkvms.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@kurtkanaskie kurtkanaskie left a comment

Choose a reason for hiding this comment

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

Given the folders and kvm files:

kvm-import-tests/
├── kvms-env
│   └── env_bu1-test_bu1-kvm2_kvmfile_0.json
├── kvms-org
│   └── org_test-org-level2_kvmfile_0.json
└── kvms-proxy
    └── proxy_bu1-test-1_bu1-test-1-kvm_kvmfile_0.json

ORG=apigeex-payg-kurt
ENV=bu1-test
PROXY=bu1-test-1

The file names structure being:
env_${ENV}${KVM}kvmfile_0.json
proxy
${$PROXY}
${KVM}kvmfile_0.json
org
${KVM}_kvmfile_0.json

I verified that the following has an error:

apigeecli kvms import --folder=./kvm-import-tests/kvms-proxy --org=$ORG

And that this works:

go run main.go -t=$TOKEN kvms import --folder=./kvm-import-tests/kvms-proxy --org=$ORG

All other imports work too.

Change whitespace to match the file: space --> tab.

@aliasgar55
Copy link
Contributor Author

Fixed the spacing issue

Copy link
Collaborator

@kurtkanaskie kurtkanaskie left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM

@ssvaidyanathan
Copy link
Collaborator

@aliasgar55 - we will be able to merge once you accept the CLA
You need to go to https://cla.developers.google.com/ and accept.

@srinandan
Copy link
Collaborator

Please run gofumpt -w . from the root folder. I think that's the last remaining issue.

@kurtkanaskie
Copy link
Collaborator

Please run gofumpt -w . from the root folder. I think that's the last remaining issue.

It took me a bit to get this installed.
First I had to upgrade go to 1.19 or greater, I used 1.21.1
Then I installed with go install mvdan.cc/gofumpt@latest which put gofump in $HOME/go/bin
Then I added that to my path.

When I ran, it flagged an empty line #42 and removed it.

@srinandan srinandan merged commit a9e08e9 into apigee:main Oct 5, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants