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

Implement CanImplement method for providers #3115

Merged
merged 4 commits into from
Apr 18, 2024
Merged

Conversation

dmjb
Copy link
Contributor

@dmjb dmjb commented Apr 16, 2024

Relates to #2845

Allows both the provider DB record as well as instances of the Provider interface to be queried to determine if they support a particular trait. Will be used more in future PRs.

Summary

Provide a brief overview of the changes and the issue being addressed.
Explain the rationale and any background necessary for understanding the changes.
List dependencies required by this change, if any.

Fixes #(related issue)

Change Type

Mark the type of change your PR introduces:

  • Bug fix (resolves an issue without affecting existing features)
  • Feature (adds new functionality without breaking changes)
  • Breaking change (may impact existing functionalities or require documentation updates)
  • Documentation (updates or additions to documentation)
  • Refactoring or test improvements (no bug fixes or new functionality)

Testing

Outline how the changes were tested, including steps to reproduce and any relevant configurations.
Attach screenshots if helpful.

Review Checklist:

  • Reviewed my own code for quality and clarity.
  • Added comments to complex or tricky code sections.
  • Updated any affected documentation.
  • Included tests that validate the fix or feature.
  • Checked that related changes are merged.

Relates to #2845

Allows both the provider DB record as well as instances of the Provider
interface to be queried to determine if they support a particular trait.
Will be used more in future PRs.
@dmjb dmjb requested a review from a team as a code owner April 16, 2024 10:33
return slices.Contains(p.Implements, impl)
}

// ProfileRow is an interface row in the profiles table
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved across from store.go - I think it's best to separate out these domain-level methods and data types

@@ -128,7 +128,7 @@ func NewProviderBuilder(

// Implements returns true if the provider implements the given type.
func (pb *ProviderBuilder) Implements(impl db.ProviderType) bool {
return slices.Contains(pb.p.Implements, impl)
return pb.p.CanImplement(impl)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have decided to leave this method for now. When my ProviderFactory PR comes along, the ProviderBuilder will be removed entirely. It doesn't seem like a good use of time to clean up all usages of this method.

@dmjb dmjb changed the title Provide CanImplement method for providers Implement CanImplement method for providers Apr 16, 2024
@coveralls
Copy link

coveralls commented Apr 16, 2024

Coverage Status

coverage: 48.16% (-0.04%) from 48.199%
when pulling 51d7d67 on provider-implements-method
into 2e88d5a on main.

@dmjb dmjb merged commit 32357b3 into main Apr 18, 2024
20 checks passed
@dmjb dmjb deleted the provider-implements-method branch April 18, 2024 09:23
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

Successfully merging this pull request may close these issues.

3 participants