Skip to content

feat: remove repo iterator#898

Closed
matthiasbruns wants to merge 22 commits into
open-component-model:mainfrom
matthiasbruns:feat/575_regex_glob_resolver_repository_2
Closed

feat: remove repo iterator#898
matthiasbruns wants to merge 22 commits into
open-component-model:mainfrom
matthiasbruns:feat/575_regex_glob_resolver_repository_2

Conversation

@matthiasbruns

Copy link
Copy Markdown
Contributor

What this PR does / why we need it

Follow up on discussions in #844

Which issue(s) this PR fixes

Contributes open-component-model/ocm-project#575

@github-actions github-actions Bot added kind/feature new feature, enhancement, improvement, extension size/m Medium labels Sep 18, 2025
@matthiasbruns matthiasbruns force-pushed the feat/575_regex_glob_resolver_repository_2 branch from 4149e72 to 818e40e Compare September 18, 2025 06:52
@github-actions github-actions Bot added the size/l Large label Sep 18, 2025
Comment thread bindings/go/repository/component/resolver/v1alpha1/repository.go Outdated
@matthiasbruns matthiasbruns force-pushed the feat/575_regex_glob_resolver_repository_2 branch from 5c3fe3d to 10049d2 Compare September 18, 2025 11:13
@matthiasbruns matthiasbruns marked this pull request as ready for review September 18, 2025 11:13
@matthiasbruns matthiasbruns requested a review from a team as a code owner September 18, 2025 11:13
@matthiasbruns matthiasbruns force-pushed the feat/575_regex_glob_resolver_repository_2 branch from 10049d2 to 5d90824 Compare September 18, 2025 11:13
@github-actions github-actions Bot added the component/github-actions Changes on GitHub Actions or within `.github/` directory label Sep 18, 2025

@fabianburth fabianburth left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we should probably also rename the whole concept from resolver to spec provider, WDYT?

Comment thread bindings/go/repository/component/resolver/v1alpha1/repository.go Outdated
Comment thread bindings/go/repository/component/resolver/v1alpha1/repository.go Outdated
Comment thread bindings/go/repository/component/resolver/v1alpha1/repository.go Outdated
@matthiasbruns

Copy link
Copy Markdown
Contributor Author

I think we should probably also rename the whole concept from resolver to spec provider, WDYT?

would this lead to a new package since this one is called repository?

@matthiasbruns

Copy link
Copy Markdown
Contributor Author

I will update the docs once the discussions are resolved

Comment thread bindings/go/repository/component/resolver/v1alpha1/repository_test.go Outdated
@fabianburth

Copy link
Copy Markdown
Contributor

I think we should probably also rename the whole concept from resolver to spec provider, WDYT?

would this lead to a new package since this one is called repository?

I believe it would

Comment thread bindings/go/repository/component/fallback/v1/doc.go Outdated
Comment thread bindings/go/repository/component/resolver/v1alpha1/repository.go Outdated
Comment thread bindings/go/repository/component/fallback/v1/doc.go Outdated
@Skarlso Skarlso changed the title feat: remove repo interator feat: remove repo iterator Sep 18, 2025
Signed-off-by: Matthias Bruns <git@matthiasbruns.com>
Signed-off-by: Matthias Bruns <git@matthiasbruns.com>
On-behalf-of: SAP <matthias.bruns@sap.com>

Signed-off-by: Matthias Bruns <git@matthiasbruns.com>
On-behalf-of: SAP <matthias.bruns@sap.com>

Signed-off-by: Matthias Bruns <git@matthiasbruns.com>
On-behalf-of: SAP <matthias.bruns@sap.com>

Signed-off-by: Matthias Bruns <git@matthiasbruns.com>
On-behalf-of: SAP <matthias.bruns@sap.com>

Signed-off-by: Matthias Bruns <git@matthiasbruns.com>
On-behalf-of: SAP <matthias.bruns@sap.com>

Signed-off-by: Matthias Bruns <git@matthiasbruns.com>
On-behalf-of: SAP <matthias.bruns@sap.com>

Signed-off-by: Matthias Bruns <git@matthiasbruns.com>
@matthiasbruns matthiasbruns force-pushed the feat/575_regex_glob_resolver_repository_2 branch from ab70523 to 5db716c Compare September 18, 2025 14:42
On-behalf-of: SAP <matthias.bruns@sap.com>

Signed-off-by: Matthias Bruns <git@matthiasbruns.com>
On-behalf-of: SAP <matthias.bruns@sap.com>

Signed-off-by: Matthias Bruns <git@matthiasbruns.com>
Comment thread bindings/go/repository/component/fallback/v1/doc.go Outdated
Comment thread bindings/go/repository/component/fallback/v1/doc.go Outdated
Comment thread bindings/go/repository/component/resolver/v1alpha1/repository.go Outdated
Comment thread bindings/go/repository/component/resolver/v1alpha1/repository_test.go Outdated
Comment thread bindings/go/repository/interface.go Outdated
matthiasbruns and others added 8 commits September 19, 2025 08:10
On-behalf-of: SAP <matthias.bruns@sap.com>

Signed-off-by: Matthias Bruns <git@matthiasbruns.com>
On-behalf-of: SAP <matthias.bruns@sap.com>

Signed-off-by: Matthias Bruns <git@matthiasbruns.com>
On-behalf-of: SAP <matthias.bruns@sap.com>

Signed-off-by: Matthias Bruns <git@matthiasbruns.com>
On-behalf-of: SAP <matthias.bruns@sap.com>

Signed-off-by: Matthias Bruns <git@matthiasbruns.com>
On-behalf-of: SAP <matthias.bruns@sap.com>

Signed-off-by: Matthias Bruns <git@matthiasbruns.com>
…m:matthiasbruns/open-component-model into feat/575_regex_glob_resolver_repository_2
@fabianburth

fabianburth commented Sep 19, 2025

Copy link
Copy Markdown
Contributor

To be honest, I'd really love to get rid of the whole resolver term for this concept and completely switch to [repository] spec provider. repository spec provider is descriptive and concise, while resolver is completely generic and could be anything - WDYT?

This would also include a renaming of the corresponding config.

@matthiasbruns

Copy link
Copy Markdown
Contributor Author

To be honest, I'd really love to get rid of the whole resolver term for this concept and completely switch to [repository] spec provider. repository spec provider is descriptive and concise, while resolver is completely generic and could be anything - WDYT?

This would also include a renaming of the corresponding config.

Current Name: SpecResolver

since its in the Repository package, we would not prepend Repository in front of the name.
I don't get the naming back and forth to be honest.

In the ticket, it was mentioned resolver. We use "ocm.software/open-component-model/bindings/go/configuration/resolvers/v1/matcher" in the SpecProvider/Resolver/WhatEver

We use the resolver package in binding. So why cant we keep the name as is? We might also add more implementations of RepositorySpecProvider

I would rather rename the interface to SpecProvider and the implementation someone like 'ResolverSpecProvider'

Otherwise feel free to rename what you think makes sense :D

On-behalf-of: SAP <matthias.bruns@sap.com>

Signed-off-by: Matthias Bruns <git@matthiasbruns.com>
…m:matthiasbruns/open-component-model into feat/575_regex_glob_resolver_repository_2
@fabianburth

Copy link
Copy Markdown
Contributor

When you began the issue, we didn't even have the repository spec provider interface. The original resolver concept is implemented completely different from the spec provider (as it implemented a component version repository). I also said that my suggestion would require to also go back and rename the configuration package.

I'm just trying to keep the naming consistent across the library, as this makes searching through it much easier too as it grows.

The interface providing component version repositories is called ComponentVersionRepositoryProvider, so I'd call the interface providing component version repository specifications ComponentVersionRepositorySpecProvider - that also provides a rather natural auto-complete experience as they will have to be used together.

Happy to hear other peoples opinions too @Skarlso @jakobmoellerdev. Might be that I care about the naming too much here.

@Skarlso

Skarlso commented Sep 19, 2025

Copy link
Copy Markdown
Contributor

I don't mind longer names as long as they make sense together with their packages names + the function name. That's how you call them right? oci repository.ComponentVersionRepositorySpecProvider.GetRepositorySpec contains a lot of Repository in it, not?

@matthiasbruns

Copy link
Copy Markdown
Contributor Author

@fabianburth can you just rename the things as you wish? I really don't mind anymore at this point :D

@jakobmoellerdev

Copy link
Copy Markdown
Member

closing in favor of #905

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component/github-actions Changes on GitHub Actions or within `.github/` directory kind/feature new feature, enhancement, improvement, extension size/l Large size/m Medium

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants