Skip to content

Conversation

estroz
Copy link
Member

@estroz estroz commented Jul 29, 2020

This PR moves pkg/apis/scorecard/v1alpha3 from operator-sdk to a central location.

/cc @joelanford @shawn-hurley @ecordell @kevinrizza

Copy link
Member

@joelanford joelanford left a comment

Choose a reason for hiding this comment

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

A couple of questions and nits.

Otherwise
/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 29, 2020
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Jul 29, 2020
@estroz estroz force-pushed the feature/scorecard-apis branch from 3bc5315 to 7bc9d3c Compare July 29, 2020 20:53
@estroz estroz requested a review from joelanford July 30, 2020 00:36
Copy link
Member

@kevinrizza kevinrizza left a comment

Choose a reason for hiding this comment

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

At a high level this seems totally reasonable. My only major question is the structure of the /pkg/ folder now. All of the OLM CRDs live in api/pkg/operators/, but now we are adding api/pkg/apis/

Should we move these apis into the operators/ folder? Or maybe move the OLM apis into their own folder in api/pkg/olm?

@estroz
Copy link
Member Author

estroz commented Jul 30, 2020

@kevinrizza following typical API package layout, shouldn’t OLM’s APIs be under pkg/apis/operators? To me it makes sense to group all APIs under pkg/apis in subpackages named for their group.

@kevinrizza
Copy link
Member

/lgtm
/approve

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 31, 2020
@estroz estroz merged commit 333d064 into operator-framework:master Jul 31, 2020
@estroz estroz deleted the feature/scorecard-apis branch July 31, 2020 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants