Skip to content

Re-merge SDK structs into main repo #10895

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

Open
jolheiser opened this issue Mar 30, 2020 · 10 comments
Open

Re-merge SDK structs into main repo #10895

jolheiser opened this issue Mar 30, 2020 · 10 comments
Labels
type/proposal The new feature has not been accepted yet but needs to be discussed first.

Comments

@jolheiser
Copy link
Member

I have prepared some sample repositories

submod represents the main Gitea repo. It has an sdk package that is also a go module.
submodder represents the Gitea go-sdk repo. It uses the sdk from submod without pulling in the entire project.

Because of the replace directive in submod's go.mod, it uses the sdk package in place of the import given.

I did some testing, and it seems that it does not run into the same vendor issues we did before, as I can update the sdk package in submod and then run go mod vendor and pick up changes.
e.g. vendoring seems to understand we want to use the sdk package directly rather than fetching some latest tag.

If I missed something, let me know, but imo it would be a huge benefit to not have to copy over structs every time we make a change to the API.
Yes, this will mean a duplication in the sense that our vendor directory will contain the same files as the sdk package, but in the scheme of things I don't know that it would add much size.


As an aside, we could maybe consider the feasibility of removing vendor and instead only providing the output of go mod vendor in release downloads, similar to how we are now doing node_modules

@jolheiser jolheiser added the type/proposal The new feature has not been accepted yet but needs to be discussed first. label Mar 30, 2020
@lunny
Copy link
Member

lunny commented Mar 31, 2020

Looks good. But there are also some others we need to disuccss.
1 If we just fixed a sdk bug which is not related with gitea, we have to publish a gitea docker and generate a gitea binary and many CI tests.
2 How to run different CI tests when sent a PR. The PR may only change sdk or server or both.

@6543
Copy link
Member

6543 commented Mar 31, 2020

interesting idear, I'll look into this later

@jolheiser
Copy link
Member Author

@lunny

They would be tagged whenever Gitea is tagged, so that the structs will always match their Gitea version counterpart.

I think maybe I mis-worded by using the term "SDK". This proposes to re-merge the structs, not the whole go-sdk repo.

@jolheiser
Copy link
Member Author

Effectively this would mean moving https://github.com/go-gitea/gitea/tree/master/modules/structs out into a base directory and naming it appropriately (I've chosen sdk but it could be something else)

Then in go-sdk we would have to rip out the structs and instead add this new module to use.

@6543
Copy link
Member

6543 commented Apr 1, 2020

to follow your deom ...

we could select witch struct we want to profide in the submodder

if we redefine it in this way?

type SDK struct sdk.SDK

so we still have track what is exposed and wat dont but dont have to make pulls like this:
https://gitea.com/gitea/go-sdk/pulls/300

@6543
Copy link
Member

6543 commented Apr 1, 2020

need more discusion ...

@jolheiser
Copy link
Member Author

There would be no need to update the structs manually in the client. You'd just update the module.
The client could just use the @latest module so it doesn't need to wait for a release cut. (releases could update module to a release cut if wanted, but that's a somewhat different discussion)

But anyone else who wants to use the sdk has the option to target a tag that would match their targeted version.

@jolheiser
Copy link
Member Author

as for your example PR, technically a PR would still need to be made to update the module.

However, it would at least automatically sync the structs, rather than the SDK not supporting a specific struct until it's manually PRd.

@lunny
Copy link
Member

lunny commented Apr 1, 2020

@jolheiser OK. I misunderstand that. If we only move structs there, it looks good.

@jolheiser
Copy link
Member Author

To clarify something that came up in Discord, the client can still have their own struct that embeds an API struct (or has it as a field) if they need further functionality for the client.

e.g.

type Issue struct {
    gitea.Issue 

    // For some reason we need to assign a client-specific ID to an Issue
    ClientID int
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/proposal The new feature has not been accepted yet but needs to be discussed first.
Projects
None yet
Development

No branches or pull requests

3 participants