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

Duplication vs common packages when adding new API version #5181

Closed
lbernick opened this issue Jul 20, 2022 · 2 comments
Closed

Duplication vs common packages when adding new API version #5181

lbernick opened this issue Jul 20, 2022 · 2 comments

Comments

@lbernick
Copy link
Member

When adding the v1 package, most of the code gets copied over from v1beta1. This makes a lot of sense for CRDs but it's less clear whether it makes sense to do for sub-components of the API that aren't CRDs, like params, workspaces, and results. This results in having to write a lot of boilerplate to convert a v1beta1 workspace into a v1 workspace (for example) even though there are no breaking changes to workspaces proposed in v1. It also means we need to maintain duplicate versions of this code, e.g. as discussed in #5171. (We'll have to maintain a lot of copy pasted code to some extent anyway, just by nature of adding a new API version, but using common definitions for parts of the API could help.)

In v1alpha1, we imported v1beta1 and used some API definitions from v1beta1 (example). This sometimes resulted in having to update v1alpha1 tests as well when we wanted to change the v1beta1 versions.

One approach is to start with using common packages for these features, and if we end up deciding to make breaking changes, swap a struct over to using copy + paste. This would break Go clients, which would be less than ideal but workable. Alternatively, we could copy and paste everything and write all the conversion boilerplate from the start.

Thoughts @tektoncd/core-maintainers?

@vdemeester
Copy link
Member

In v1alpha1, we imported v1beta1 and used some API definitions from v1beta1 (example). This sometimes resulted in having to update v1alpha1 tests as well when we wanted to change the v1beta1 versions.

Those are type alias, and we did set them to have to not have to use v1beta1 package in v1alpha1 though 🤔.

I am usually always on the side of "a little duplication is better than the wrong abstraction". Old API code are meant to be removed at some point, and, once we fully support v1, we should consider v1beta1 as frozen (meaning.. those struct shouldn't change at any point). Sharing code between the two is a bit error-prone as we may not realize we are breaking or changing v1beta1 until we face issues (after a release).
In a gist, I tend to prefer the copy/paste approach for this.

Also, from a pure consumption perspective, if a project generates structures from our API, it will do the same, aka have two completely un-related codebase for v1beta1 and v1.

@lbernick
Copy link
Member Author

Makes sense to me! I went with the duplication strategy in #5202. Going to close but anyone can feel free to reopen to continue the discussion.

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

No branches or pull requests

2 participants