-
Notifications
You must be signed in to change notification settings - Fork 31
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
Remove footer feature flag #1450
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And save 2 lines of code? 🤔 I'm not quite sure of the benefit of this. And as long as we don't have any other checks to make sure the backend version is supported, I'm not sure we should just pull out all the stops.
I thought we had agreed to have a minimum supported backend version? The feature flags are nothing more than a guard against this, so they serve no purpose if we do that. |
Possibly (I can't recall specifics), but I think we should implement some kind of sanity check in that case, and warn app developers in the frontend immediately if their backends are not updated. And I also believe the feature flags from backend were supposed to be just that; flags that could be turned off. Although I guess for this specific feature it just guards against us calling an API endpoint that didn't exist in previous versions. So, what happens after this change - will older app-backends produce the 'unknown error' if the footer request fails? In that case, I agree that removing the feature flag check is a good thing. |
Yes this one is hardcoded to
Not unless we also remove the |
Yep, do it! 🙌 Old backend = fatal error. That's much better than old backend = multiple fixed bugs/issues creeping back to the surface. |
Kudos, SonarCloud Quality Gate passed! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, though I had approved it! 🚀
Tested OK. Although our |
Description
Assuming v4 requires app-lib version >= 7.6.0, we can remove the featureflag check for footer.
Related Issue(s)
Verification/QA
kind/*
label to this PR for proper release notes grouping