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

refactor(app-shell): remove old feature flags #602

Merged
merged 2 commits into from
May 3, 2019

Conversation

tdeekens
Copy link
Contributor

@tdeekens tdeekens commented May 3, 2019

Summary

I assumed flags are passed from the app but quick-access connects to them itself. Another case for types casue the prop-types are not followed through to the create-commands-fn.

@tdeekens tdeekens self-assigned this May 3, 2019
@tdeekens tdeekens added Semver: FIX 🐛 Type: Bug Something isn't working labels May 3, 2019
@tdeekens tdeekens requested a review from adnasa May 3, 2019 15:21
'customerGroups',
'projectSettings',
'developerSettings',
'projectSettingsChannels',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I went ahead and did a little clean up. These feature won't be untoggled and we should clean up our used flags in the MC then too.

@tdeekens tdeekens changed the title fix(app-shell): pass load feature flag refactor(app-shell): remove old feature flags May 3, 2019
Copy link
Contributor

@adnasa adnasa left a comment

Choose a reason for hiding this comment

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

perfect!
it looks to me that I have to update this file as well, since I introduced 2 new feature flags?

@tdeekens
Copy link
Contributor Author

tdeekens commented May 3, 2019

it looks to me that I have to update this file as well, since I introduced 2 new feature flags

No. Only if those features are ought to be accessible through Quick Access and their access is toggled.

@adnasa
Copy link
Contributor

adnasa commented May 3, 2019

yeah I just realised that now. carry on 😗

@tdeekens tdeekens force-pushed the fix/passing-projects-list branch from fd3603c to 4f76f71 Compare May 3, 2019 15:33
},
featureToggles.customApplicationsSettings &&
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was actually wrong. There is no flag like this and the flag requested is called differently. Hence, quick access for custom app settings was never working to how I see it.

@tdeekens tdeekens merged commit 79b1058 into master May 3, 2019
@tdeekens tdeekens deleted the fix/passing-projects-list branch May 3, 2019 15:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 Type: Bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants