open your objects for extension

[tynutsu]

this is a very weird model which has business logic coupled with pojo, setters should not actually perform the github rest calls, instead a dedicated service should do that.

while i understand is a lot work to re-model, what you could do at least is to allow the classes in the library to be further extended, so they should not be sealed or marked as final.

[bitwiseman]

@tynutsu I agree that having getters and setters making direct calls can be quite confusing - which getters retrieve a local field vs which make rest calls. The same for setters. Changing this would be a large undertaking, basically reimplementing most of the library with cleaned up methods and design.

By separate service, you mean another class that you pass the objects to and that class makes the appropriate REST call, correct? Or do you mean something else?

I have thought about implementing a green-field reimplementation of this library, but I have not have the bandwidth do work on it.

As to making the classes extendable, that trades one set of problems for another even harder set of problems. The only reason I've been able to continue to maintain and improve this library's structure up to this point is by knowing those changes will not break existing users. Making everything that is currently package internal stable enough for general extendibility would be a large amount of work.

What are you trying to do that requires the classes to be extended? Perhaps that functionality can be pulled into this library.

If that is not an option, are you interested in committing resources to making the needed changes, including testing and responding to user issues related to those changes?

[tynutsu]

Changing access modifier would allow adding wrappers around your models and further extension with real getter and setter functionality. I use this library to chain commands and setup repositories with configurable default settings (create org/repo, initialize repo, set merge strategy, create team, bind team to repo, create branch, setup default branch policies, etc.) I also think it would be easier to rewrite the whole thing with a new design, since the github API is not that huge. But I think you should wait for v4 which hopefully will encapsulate all the preview APIs with different headers, and you would be able to set up a repo with one request instead of multiple (different call to set visibility, different call to make it auto init, etc.) Once v4 is out I'll see if I have cycles to contribute.

[mxandeco]

The "rich domain" is quite useful when implementing Webhooks, when for example handling a pull_request opened call, it's much cleaner and straightforward to be able to call getPullRequest().addLabel() instead of having to inject a different service object to perform the task.

Even though if standalone pojos and services are to be created, keeping the current models, especially for the events would be good imo.

[tynutsu]

The "rich domain" is quite useful when implementing Webhooks, when for example handling a pull_request opened call, it's much cleaner and straightforward to be able to call getPullRequest().addLabel() instead of having to inject a different service object to perform the task.

Even though if standalone pojos and services are to be created, keeping the current models, especially for the events would be good imo.

Although I totally disagree even with combining 2 services' business logic, since it's against Single Responsibility Principle (LabelService and PullRequestService), this is why I asked to open them for extension, to not break existing users implementation.

[bitwiseman]

@tynutsu

Changing access modifier would allow adding wrappers around your models and further extension with real getter and setter functionality.

Perhaps I'm not understanding... I don't seen that many final class declarations. Changing those would be fine with me.

There are a bunch of package-private and those are generally intentionally so, but I'd be open to looking at changing those to public on a case-by-case basis. Which classes in particular are you talking about?

But I think you should wait for v4 which hopefully will encapsulate all the preview APIs with different headers, and you would be able to set up a repo with one request instead of multiple (different call to set visibility, different call to make it auto init, etc.)

Perhaps you could file a issues for specific things you want to see. For example, you mentioned wanting to have one call to create a repo that includes setting visibility and auto-init, etc - that behavior is currently supported by the GitHub REST API. All that would be needed would be for someone to implement it in this library. In fact, there's already a GHRepositoryBuilder that does most of what you mentioned. It looks like it only needs some an additional setting for auto-init.

I also think it would be easier to rewrite the whole thing with a new design, since the github API is not that huge.

But on the other hand...

Once v4 is out I'll see if I have cycles to contribute.

ಠ_ಠ A full rerwrite is always easier when it is done by someone else.

@mxandeco

The "rich domain" is quite useful when implementing Webhooks, when for example handling a pull_request opened call, it's much cleaner and straightforward to be able to call getPullRequest().addLabel() instead of having to inject a different service object to perform the task.

Even though if standalone pojos and services are to be created, keeping the current models, especially for the events would be good imo.

We could probably keep the existing implementation and have it wrap the new one.

[tynutsu]

The main classes i care about are GHOrganization, GHRepository, GHBranch, GHTeam so far (I'm enumerating this by heart). But the lib lacks enterprise api (ability to create org, delete org, set up enterprise permissions, etc.) and probably the gitHub client itself. Also can't find a simple way to send a request to a custom url not covered by the library (/v3/api/gitignore/templates, /v3/api/admin/organizations, /v3/api/enterprise/stats/$type, etc.) So opening the github client itself to allow createPost createDelete createGet etc, would be nice (in this case i might not actually know how to do it, the library might be able to do it, i just don't know how)

[HiranChaudhuri]

I tried to extend from GHObject so I could have some of my own objects feed into a homoenous list of GHObjects. I failed miserably and believe the root cause to be somehwere between GHObject and OpenJDK. See also https://stackoverflow.com/questions/72341093/puzzling-class-is-not-abstract-and-does-not-override-abstract-method