Page MenuHomePhabricator

[native] hardcode avatar feature flag to true
ClosedPublic

Authored by ginsu on Apr 26 2023, 4:52 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Oct 30, 12:52 PM
Unknown Object (File)
Wed, Oct 30, 12:52 PM
Unknown Object (File)
Wed, Oct 30, 12:52 PM
Unknown Object (File)
Wed, Oct 30, 12:52 PM
Unknown Object (File)
Wed, Oct 30, 12:51 PM
Unknown Object (File)
Tue, Oct 1, 10:28 PM
Unknown Object (File)
Oct 1 2024, 12:40 PM
Unknown Object (File)
Sep 14 2024, 11:17 PM
Subscribers

Details

Summary

This is a naive/quick approach to hardcoding the avatar feature flag to true. Ideally we should not be using a hook anymore, but making that change would require me to go to every place this feature flag is used and update it accordingly. I personally felt that was a bit unnecessary especially after we talked today about deprecating all the avatar feature flags in the codebase sometime soon and this would take care of this using a unnecessary hook concern. (ENG-3413 considers removing and deprecating the feature flag rendering logic throughout our codebase)

Test Plan

Avatars now always renders on native

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

ginsu added reviewers: ashoat, atul.
ginsu edited the summary of this revision. (Show Details)
ginsu requested review of this revision.Apr 26 2023, 5:10 PM
This revision is now accepted and ready to land.Apr 27 2023, 10:16 AM

I don't think this is the best approach - ideally, we should change feature flags configuration by setting the flag for everyone (with appropriate code versions) to true. The benefit is that if we discover a serious issue with the feature, we could quickly disable it. After it is hardcoded, we will have to release a new version for that purpose.

My guess is that we wanted to do it quickly, and that's why we decided to do it that way, but maybe there are other reasons which I don't see at this point.

Sorry, this should have been explained in the diff. The main reason we're doing it this way are the issues in ENG-3252. Many of us (myself included) are currently unable to use the avatars feature because the request consistently times out when we open the app

Sorry, this should have been explained in the diff. The main reason we're doing it this way are the issues in ENG-3252. Many of us (myself included) are currently unable to use the avatars feature because the request consistently times out when we open the app

Ah, ok, makes a lot of sense!