awx-go
awx-go copied to clipboard
Colstuwjx
WIP: Add changes from dahendel & co
Unfortunately the repository that Dustin Hendel is maintaining on GitLab is not sharing the same history. So I had to duplicate code with meld. I stripped the changes that were not relevant (such as disabling tests ...)
Colstuwjx:master and davidfischer-ch:master are entirely different commit histories.
Please can you review it, and adapt tests accordingly. Unfortunately I don't have (yet) much experience in go.
Best Regards,
David
Thanks for your PR!
I leave some comments for requesting changes. BTW, why do you disable the test cases teams_test.go and credentials_test.go?
Hello,
I forgot to enable tests that were disabled when copying files from the contributors.
I think you have the right to modify the PR, I invite you to do the modifications because you are better in judging the changes.
Do you agree?
Thanks for your PR!
I leave some comments for requesting changes. BTW, why do you disable the test cases
teams_test.goandcredentials_test.go?
I just enabled those tests.
Some issues I have trying to test based on the steps in Travis Config:
david@ZenBookPro:~/develop/awx-go$ make lint
gometalinter: error: unknown linters: gosimple
Makefile:27: recipe for target 'lint' failed
make: *** [lint] Error 1
david@ZenBookPro:~/develop/awx-go$ ./codecov.sh
../../go/src/_/home/david/develop/awx-go/job_template.go:10:2: cannot find package "github.com/twinj/uuid" in any of:
/home/david/go/src/_/home/david/develop/awx-go/vendor/github.com/twinj/uuid (vendor tree)
/usr/lib/go-1.10/src/github.com/twinj/uuid (from $GOROOT)
/home/david/go/src/github.com/twinj/uuid (from $GOPATH)
I suppose go.mod has to be updated? What is the best practice?
If you can take time to explain me how, I will be your Padawan apprentice.
Obviously same issue for make build, etc.
david@ZenBookPro:~/develop/awx-go$ make lint
gometalinter: error: unknown linters: gosimple
Makefile:27: recipe for target 'lint' failed
make: *** [lint] Error 1
The gometalinter repo has been archived, and I will make a PR to change to use golangci-lint, you could skip this part, since it's been called by travis CI right now.
I suppose go.mod has to be updated? What is the best practice? If you can take time to explain me how, I will be your Padawan apprentice.
For this issue, yes, you need to update the go.mod as well.
It seems that you've NOT grant me write access to your branch, so I'd like to just leave the go mod fix code in this branch, see this commit.
What's more, you should also make the make test works, the struct def is also not good to me:
>> running all tests
# github.com/Colstuwjx/awx-go [github.com/Colstuwjx/awx-go.test]
./credentials_test.go:13:5: unknown field 'Type' in struct literal of type Credential
./credentials_test.go:14:5: unknown field 'URL' in struct literal of type Credential
./credentials_test.go:15:5: unknown field 'Related' in struct literal of type Credential
./credentials_test.go:26:5: unknown field 'SummaryFields' in struct literal of type Credential
./credentials_test.go:62:5: unknown field 'Created' in struct literal of type Credential
./credentials_test.go:66:5: unknown field 'Modified' in struct literal of type Credential
./credentials_test.go:72:5: unknown field 'Organization' in struct literal of type Credential
./credentials_test.go:73:5: unknown field 'CredentialType' in struct literal of type Credential
./credentials_test.go:74:5: unknown field 'Inputs' in struct literal of type Credential
./credentials_test.go:82:23: awx.CredentialService undefined (type *AWX has no field or method CredentialService)
./credentials_test.go:82:23: too many errors
FAIL github.com/Colstuwjx/awx-go [build failed]
? github.com/Colstuwjx/awx-go/awxtesting/mockserver [no test files]
? github.com/Colstuwjx/awx-go/awxtesting/mockserver/mockdata [no test files]
FAIL
make: *** [test] Error 2
I suggest that align with the current model struct definitions. Let me know if you need more helps, I will be glad to give a hand. Thanks.
I don't know exactly how you can contribute to the PR, however, I enabled this option since the beginning, in the meantime, I just merged your changes to my branch, thanks!
@Colstuwjx this is a work in progress. I am enhancing tests and making many changes, I'll keep you informed in case I have questions or need your expertise.
Thank you.
I don't know exactly how you can contribute to the PR, however, I enabled this option since the beginning, in the meantime, I just merged your changes to my branch, thanks!
It maybe my mistake, I'll be here if you have any questions or need help.
@Colstuwjx this is a work in progress. I am enhancing tests and making many changes, I'll keep you informed in case I have questions or need your expertise.
Sure, welcome!
Current status : All (enabled) tests pass. However I have to add more mock data to enable remaining tests.
See https://github.com/davidfischer-ch/awx-go/commit/6049f8c0e2ae7a4e90c17189df07e874b9b7b62e.
I am wondering if you have some time to give a hand on this?
Thanks.
Yet another remark: https://github.com/Colstuwjx/awx-go/pull/33/commits/6f91b035381755efc8918adbc82dcce02507b051 is changing behavior, because the update methods are now using PATCH instead of PUT to allow partial updates. Of course full updates are still possible.
Yet another remark: 6f91b03 is changing behavior, because the update methods are now using PATCH instead of PUT to allow partial updates. Of course full updates are still possible.
For the change behavior, I suggest move them into another separate PR, we should NOT make this PR too robust, let's just focus on introduce more AWX resources and their tests in this PR.
Current status : All (enabled) tests pass. However I have to add more mock data to enable remaining tests.
See davidfischer-ch@6049f8c.
I am wondering if you have some time to give a hand on this?
Okay.
Yet another remark: 6f91b03 is changing behavior, because the update methods are now using PATCH instead of PUT to allow partial updates. Of course full updates are still possible.
For the change behavior, I suggest move them into another separate PR, we should NOT make this PR too robust, let's just focus on introduce more AWX resources and their tests in this PR.
Current status : All (enabled) tests pass. However I have to add more mock data to enable remaining tests. See davidfischer-ch/awx-go@6049f8c. I am wondering if you have some time to give a hand on this?
Okay.
Change behavior has now its own PR https://github.com/Colstuwjx/awx-go/pull/34