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

Provider, Resource, and DataSource types #32

Merged
merged 5 commits into from
Jun 2, 2021
Merged

Provider, Resource, and DataSource types #32

merged 5 commits into from
Jun 2, 2021

Conversation

kmoe
Copy link
Member

@kmoe kmoe commented Jun 1, 2021

closes #14
closes #15

@kmoe kmoe requested a review from a team June 1, 2021 18:33
@paddycarver paddycarver added this to the v0.1.0 milestone Jun 1, 2021
Copy link
Contributor

@paddycarver paddycarver left a comment

Choose a reason for hiding this comment

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

Some minor nitpicks and suggestions, but this looks pretty much right to me.

resource.go Outdated Show resolved Hide resolved
data_source.go Outdated Show resolved Hide resolved
data_source.go Outdated
)

// A DataSourceType is a type of data source. For each type of data source this
// provider supports, it should instantiate a struct implementing DataSourceType
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// provider supports, it should instantiate a struct implementing DataSourceType
// provider supports, it should define a type implementing DataSourceType

Copy link
Member Author

Choose a reason for hiding this comment

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

If we clarify in this direction, I'd rather we change the whole thing to "it should define a type implementing DataSourceType and return an instance [of that type] in the map returned by Provider.GetDataSources." Will do so and we can see if it reads better.

provider.go Outdated Show resolved Hide resolved
provider.go Show resolved Hide resolved
resource.go Outdated
)

// A ResourceType is a type of resource. For each type of resource this provider
// supports, it should instantiate a struct implementing ResourceType and return
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// supports, it should instantiate a struct implementing ResourceType and return
// supports, it should define a type implementing ResourceType and return

resource.go Outdated Show resolved Hide resolved
@kmoe kmoe requested a review from paddycarver June 2, 2021 11:26
Copy link
Contributor

@paddycarver paddycarver left a comment

Choose a reason for hiding this comment

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

👍 Let's do it 🚀

@kmoe kmoe merged commit ddd8fa5 into main Jun 2, 2021
@kmoe kmoe deleted the provider_type branch June 2, 2021 15:03
paddycarver added a commit that referenced this pull request Jun 2, 2021
paddycarver added a commit that referenced this pull request Jun 2, 2021
paddycarver added a commit that referenced this pull request Jun 2, 2021
@github-actions
Copy link

github-actions bot commented Jul 3, 2021

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Define resource and data source types Define provider type.
2 participants