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 ServicePayload to remove nested Service attribute from Service struct #58

Merged
merged 1 commit into from
Sep 17, 2021

Conversation

pdecat
Copy link
Contributor

@pdecat pdecat commented Aug 20, 2021

This PR resolves #57

@stmcallister
Copy link
Collaborator

Totally agree with this change. This is how we design resources today. Just one ask before approving. Will you also fix the service_test.go? I received the following error when I ran the tests. Thanks!

==> Checking that code complies with gofmt requirements... ✔
==> Checking that code complies with go vet requirements...# github.com/heimweh/go-pagerduty/pagerduty
vet: pagerduty/service_test.go:41:27: v.Service undefined (type *Service has no field or method Service)
 ✔
==> Checking for misspelling errors... ✔
==> Checking that code complies with staticcheck requirements...pagerduty/service_test.go:41:27: v.Service undefined (type *Service has no field or method Service) (compile)
pagerduty/service_test.go:111:27: v.Service undefined (type *Service has no field or method Service) (compile)
make: *** [staticcheck] Error 1

@heimweh
Copy link
Owner

heimweh commented Aug 25, 2021

Nice job @pdecat! Really appreciate it! Been wanting to tackle this for some time now 😅
Unfortunately, we have more examples of this scattered across the codebase that I'd like to clean up as well.
Let's use this and the new resources as an example going forward and try to align the existing resources the same way 👍

@pdecat pdecat force-pushed the remove_service_in_service branch from 86e196a to 9511f75 Compare August 26, 2021 06:13
@pdecat
Copy link
Contributor Author

pdecat commented Aug 26, 2021

Fixed the tests:

# make test
==> Checking that code complies with gofmt requirements... ✔
==> Checking that code complies with go vet requirements... ✔
==> Checking for misspelling errors... ✔
==> Checking that code complies with staticcheck requirements... ✔
==> Checking that code complies with golint requirements... ✔
==> Checking that code complies with unit tests...
ok      github.com/heimweh/go-pagerduty/pagerduty       0.048s  coverage: 62.3% of statements

@pdecat
Copy link
Contributor Author

pdecat commented Aug 26, 2021

Submitted #61 to enable Github Actions to run them automatically.

Copy link
Collaborator

@stmcallister stmcallister left a comment

Choose a reason for hiding this comment

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

Awesome! Thank you!! 🎉

@pdecat pdecat force-pushed the remove_service_in_service branch from 9511f75 to c3f4857 Compare September 10, 2021 09:06
@heimweh heimweh merged commit f48300a into heimweh:master Sep 17, 2021
@pdecat pdecat deleted the remove_service_in_service branch September 17, 2021 22:12
stmcallister pushed a commit that referenced this pull request Oct 21, 2021
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.

Service struct should not contain a Service attribute
3 participants