Remove embedded APIObject from struct types, since it's not inheritance

[theckman]

The V1 API currently tries to use embedding as inheritance by putting the APIObject inside of each type to avoid duplicating fields, and it's caused a few issues that will require us to make breaking changes to fix (#218 #316). We will need to find and fix these issues for v1.5.0, and for v2.0.0 we should remove the embeds completely and flatten the structures by hand.

[whereswaldon]

So I'm just a random user with far less context than you have on this problem, so please take this as just an uninformed opinion: Your use of embedding here seems like a fine use case of composition, not inheritance. Grouping shared common fields into a subtype seems like a good idea.

From my outsider perspective, the problem is that the outer types that embed the APIObject duplicate some of its fields. I would think the fix is to remove the Id, Type and other shared fields from the outer types, not removing the APIObject type. I've read the issues that you linked, and I don't see a reason that removing the duplicated fields from the outer objects wouldn't resolve the issue.

Regardless, thank you for trying to maintain this codebase. I saw in another issue that you aren't even a PD employee anymore, and I would understand if you left it to rot. Maintainer's guilt is hard to shake, but sometimes you gotta to stay healthy.

So yeah, if you decide to try for a v2 API, I hope the above is helpful, and I'm sorry if I'm spewing nonsense because I don't understand the constraints you're solving for. Thanks again for your hard work!

[theckman]

Regardless, thank you for trying to maintain this codebase. I saw in another issue that you aren't even a PD employee anymore, and I would understand if you left it to rot. Maintainer's guilt is hard to shake, but sometimes you gotta to stay healthy.

@whereswaldon not even maintainer's guilt, my buddy started this project while he was there. I'm carrying his guilt, jk. 😂

The half-truth is that I introduced Go at PagerDuty so I feel semi-responsible for him even starting the project (😛 ), but also I'm a user of it for my job at Netflix.

Your use of embedding here seems like a fine use case of composition, not inheritance. Grouping shared common fields into a subtype seems like a good idea.

Thank you for sharing your thoughts on this, although I'm not sure I'm aligned with your perspective. Specifically, I've only ever seen composition used to describe a has-a relationship, while inheritance has been there to describe an is-a relationship.

In relation to the PagerDuty API, some of the struct types we've defined are meant to represent an API object returned from the REST API. Said another way, one of those structs is an API object, it does not have an API object. As such I don't believe composition is the right tool to use, because we're not trying to represent a has-a relationship with the API object.

This is one of the pains of composition (embedding) supporting promotion of methods and fields, because it makes it look like inheritance. So powerful, but with great power comes great responsibility. That was definitely abused in the early version of this package and so I think we should address that for v2+. I say this while still in regular contact with the person who initially put APIObject in this repo. :smile:

[whereswaldon]

Thank you for sharing your thoughts on this, although I'm not sure I'm aligned with your perspective. Specifically, I've only ever seen composition used to describe a has-a relationship, while inheritance has been there to describe an is-a relationship.

You're welcome. I think that what you're saying is true of composition within OOP languages, but those also don't automatically promote their fields in Java/C++/friends. I think that composition in Go is designed to be flexible enough to represent both relationships. You embed types for an is-a relationship, and you give those fields names (so that their members are not promoted) for a has-a relationship.

That being said, it is your call. I'll be happy with a maintained package no matter what the outcome. Thanks for reading, and for keeping up with this package!