Skip to content

feat: allow know the project type by the config file#3330

Merged
camilamacedo86 merged 2 commits intooperator-framework:masterfrom
camilamacedo86:run-local-for-new
Jul 10, 2020
Merged

feat: allow know the project type by the config file#3330
camilamacedo86 merged 2 commits intooperator-framework:masterfrom
camilamacedo86:run-local-for-new

Conversation

@camilamacedo86
Copy link
Copy Markdown
Contributor

@camilamacedo86 camilamacedo86 commented Jul 1, 2020

Description of the change:

  • Make ProjectTypes helpers looking for the type in the PROJECT file when it exists.

Motivation for the change:

  • To allow the SDK CLI commands works properly for the new layout
  • Follow up the same standard for all types shows very beneficial and increase the manutence ability

@camilamacedo86 camilamacedo86 added kubebuilder-integration Relates to rewriting the SDK in Kubebuilder plugin form language/ansible Issue is related to an Ansible operator project language/helm Issue is related to a Helm operator project labels Jul 1, 2020
@camilamacedo86 camilamacedo86 requested review from estroz and hasbro17 July 1, 2020 21:20
}
return ""
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

move the implementation to the project util since is required/useful to check the project type.

Comment thread internal/util/projutil/project_util.go
Comment thread internal/util/projutil/project_util.go
Copy link
Copy Markdown
Member

@asmacdo asmacdo left a comment

Choose a reason for hiding this comment

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

Some of this logic could be DRYer, but while we support the legacy layouts, this is fine.

Comment thread internal/util/projutil/project_util.go Outdated
Comment thread internal/util/projutil/project_util.go Outdated
Comment thread internal/util/projutil/project_util.go Outdated
Comment thread internal/util/projutil/project_util.go Outdated
}
return PluginKeyToOperatorType(cfg.Layout) == OperatorTypeAnsible
}
// todo(camilamacedo86): remove when the legacy layout is no longer supported
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Will the roles dir no longer be in the new layout?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The roles dir will be in the new layout, however, see:

  • Follow up the same standard for all types shows very beneficial and increase the manutence ability
  • We can init an Ansible/Helm project without scaffold an API which means that the roles dir would NOT exist.

}
return PluginKeyToOperatorType(cfg.Layout) == OperatorTypeHelm
}
// todo(camilamacedo86): remove when the legacy layout is no longer supported
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Will the charts dir no longer be in the new layout?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ditto above. We can scaffold a project without any api.

Comment thread internal/util/projutil/project_util.go
Comment thread internal/util/projutil/project_util.go Outdated
Comment thread internal/util/projutil/project_util.go Outdated
@camilamacedo86 camilamacedo86 requested a review from estroz July 7, 2020 09:24
@joelanford
Copy link
Copy Markdown
Member

@camilamacedo86 now that we have #3363 and #3371, is this PR still needed?

@camilamacedo86
Copy link
Copy Markdown
Contributor Author

camilamacedo86 commented Jul 9, 2020

Hi @joelanford,

The changes to allow the project type helpers get the type based in the config file for the new layout still required.
I updated the PR. Feel free to check it.

@camilamacedo86 camilamacedo86 changed the title Run local for new layout feat: allow know the project type by the config file Jul 9, 2020
Comment thread internal/util/projutil/project_util.go Outdated
Copy link
Copy Markdown
Member

@estroz estroz left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 10, 2020
Co-authored-by: Eric Stroczynski <estroczy@redhat.com>
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Jul 10, 2020
@openshift-ci-robot
Copy link
Copy Markdown

New changes are detected. LGTM label has been removed.

@camilamacedo86 camilamacedo86 merged commit 91d5687 into operator-framework:master Jul 10, 2020
@camilamacedo86 camilamacedo86 deleted the run-local-for-new branch July 10, 2020 20:25
joelanford pushed a commit to joelanford/operator-sdk that referenced this pull request Jul 13, 2020
…ork#3330)

**Description of the change:**
- Make ProjectTypes helpers looking for the type in the PROJECT file when it exists. 

**Motivation for the change:**

- To allow the SDK CLI commands works properly for the new layout 
- Follow up the same standard for all types shows very beneficial and increase the manutence ability
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kubebuilder-integration Relates to rewriting the SDK in Kubebuilder plugin form language/ansible Issue is related to an Ansible operator project language/helm Issue is related to a Helm operator project

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants