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

for helm/ansible fix msg for multigroup support #3822

Merged
merged 7 commits into from
Sep 15, 2020
Merged

for helm/ansible fix msg for multigroup support #3822

merged 7 commits into from
Sep 15, 2020

Conversation

camilamacedo86
Copy link
Contributor

@camilamacedo86 camilamacedo86 commented Sep 1, 2020

Description of the change:
Ansible/Helm fix mensagem for multigroup support

Motivation for the change:

Checklist

If the pull request includes user-facing changes, extra documentation is required:

@camilamacedo86 camilamacedo86 requested review from bharathi-tenneti, theishshah and varshaprasad96 and removed request for estroz September 1, 2020 10:57
@camilamacedo86 camilamacedo86 added this to the v1.1.0 milestone Sep 1, 2020
@camilamacedo86 camilamacedo86 added kind/feature Categorizes issue or PR as related to a new feature. and removed kind/feature Categorizes issue or PR as related to a new feature. labels Sep 1, 2020
@camilamacedo86 camilamacedo86 changed the title feat: for helm/ansible add support to multigroup by default bugfix: for helm/ansible fix msg for multigroup support Sep 13, 2020
@camilamacedo86 camilamacedo86 added the kind/bug Categorizes issue or PR as related to a bug. label Sep 13, 2020
@camilamacedo86
Copy link
Contributor Author

Hi @joelanford,

The problem is solved as suggested. By justing changing the msg.

Feel free to check.

internal/plugins/ansible/v1/scaffolds/api.go Outdated Show resolved Hide resolved
internal/plugins/helm/v1/scaffolds/api.go Outdated Show resolved Hide resolved
changelog/fragments/ansible-helm-multigroup.yaml Outdated Show resolved Hide resolved
@camilamacedo86 camilamacedo86 changed the title bugfix: for helm/ansible fix msg for multigroup support for helm/ansible fix msg for multigroup support Sep 14, 2020
@@ -88,8 +87,7 @@ func (s *apiScaffolder) scaffold() error {

// Check that the provided group can be added to the project
if !s.config.MultiGroup && len(s.config.Resources) != 0 && !s.config.HasGroup(resourceOptions.Group) {
return fmt.Errorf("multiple groups are not allowed by default, to enable multi-group visit %s",
"kubebuilder.io/migration/multi-group.html")
return errors.New("support for multiple groups is disabled; to enable, set `multigroup: true` in the PROJECT file")
Copy link
Member

Choose a reason for hiding this comment

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

For consistency with the Go error:

Suggested change
return errors.New("support for multiple groups is disabled; to enable, set `multigroup: true` in the PROJECT file")
return errors.New("multiple groups are not allowed by default, to enable multi-group set 'multigroup: true' in your PROJECT file")

internal/plugins/helm/v1/scaffolds/api.go Outdated Show resolved Hide resolved
Copy link
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 Sep 14, 2020
@camilamacedo86 camilamacedo86 dismissed joelanford’s stale review September 15, 2020 15:31

@joe i am moving forward with since all your changes were suggested and it is very tiny/small

@camilamacedo86 camilamacedo86 merged commit 5ab7236 into operator-framework:master Sep 15, 2020
@camilamacedo86 camilamacedo86 deleted the multigroup branch September 15, 2020 15:32
@camilamacedo86 camilamacedo86 removed the kind/bug Categorizes issue or PR as related to a bug. label Sep 15, 2020
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.

ansible/helm should be multigroup by default
4 participants