Skip to content
This repository has been archived by the owner on Sep 5, 2024. It is now read-only.

Remove Settings.product.transformation - no longer conditional on a product feature #490

Merged
merged 1 commit into from
Jul 16, 2018
Merged

Remove Settings.product.transformation - no longer conditional on a product feature #490

merged 1 commit into from
Jul 16, 2018

Conversation

himdel
Copy link
Contributor

@himdel himdel commented Jul 16, 2018

We're using rbac now, from #417

Fixes #432

@Fryguy , @priley86 I have no idea about the gaprindashvili status of this? Should the new builds have it enabled too, or is the product feature removal hammer only?

@himdel
Remove Settings.product.transformation - no longer conditional on a p…
1a8fd52 
…roduct feature

We're using rbac now, from #417

Fixes #432
@himdel himdel mentioned this pull request Jul 16, 2018
Merged
@priley86
Copy link
Member

priley86 commented Jul 16, 2018 •
edited
Loading

i believe this was also slated for G... ping @bthurber ...

@AllenBW
Copy link
Member

AllenBW commented Jul 16, 2018

We should hold off on merging this one till ManageIQ/manageiq#17711 gets in.

@himdel
Copy link
Contributor Author

himdel commented Jul 16, 2018

@AllenBW No no, it needs to be the other way around.

ManageIQ/manageiq#17711 can't be merged until this one is, otherwise v2v will just disappear :).

AllenBW
AllenBW approved these changes Jul 16, 2018
View reviewed changes
Copy link
Member

@AllenBW AllenBW left a comment

Choose a reason for hiding this comment

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

RBAC is wonderful! <3 👍

@AllenBW
Copy link
Member

AllenBW commented Jul 16, 2018

Merging, looks to be working just peachy! (user missing product feature, doesn't see Migration nav element)

screen shot 2018-07-16 at 10 00 55 am

@AllenBW AllenBW merged commit 0c5c506 into ManageIQ:master Jul 16, 2018
@himdel himdel deleted the remove-product branch July 16, 2018 14:22
@AparnaKarve
Copy link
Contributor

@simaishi The gaprindashvili/yes label here and in ManageIQ/manageiq#17711 is questionable.
(not sure who makes the decision on that)

I think the changes in this PR are meant for master only.

Before backporting this PR and ManageIQ/manageiq#17711, let us first find out what exactly has been planned for the Settings.product.transformation flag.
Thanks.

@himdel
Copy link
Contributor Author

himdel commented Jul 18, 2018

Answered in ManageIQ/manageiq#17711 (comment), the label is correct.

@himdel himdel mentioned this pull request Jul 18, 2018
Merged
@himdel
Copy link
Contributor Author

himdel commented Jul 20, 2018

OK, so it's not after all, sorry :).

ManageIQ/manageiq#17711 did not go in gaprindashvili,
only ManageIQ/manageiq#17733 will.

So, keeping with that, keeping the flag logic for gaprindashvili and only removing in master.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@AllenBW AllenBW AllenBW approved these changes

@martinpovolny martinpovolny martinpovolny approved these changes

Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@himdel @priley86 @AllenBW @AparnaKarve @martinpovolny