Page MenuHomePhabricator

[lib] introduce getRandomDefaultEmojiAvatar function
ClosedPublic

Authored by ginsu on Apr 12 2023, 11:49 AM.
Tags
None
Referenced Files
F6182764: D7401.id.diff
Wed, Apr 23, 8:08 AM
F6167997: D7401.id25069.diff
Wed, Apr 23, 1:06 AM
F6165919: D7401.id25072.diff
Tue, Apr 22, 11:35 PM
Unknown Object (File)
Tue, Apr 22, 5:14 AM
Unknown Object (File)
Sat, Apr 19, 2:48 PM
Unknown Object (File)
Wed, Apr 9, 1:05 AM
Unknown Object (File)
Wed, Apr 9, 12:06 AM
Unknown Object (File)
Tue, Apr 8, 11:15 PM
Subscribers

Details

Summary

small utiltity function that will be used in the following diff

Test Plan

Tested in a subsequent diff locally that I was getting a random default avatar on return when calling the function

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

I would have included this in D7402. It's hard to understand the need for this function (vs. the existing hash function approach) without reading your comment here. In general I think small utility functions like this should be introduced alongside their callsite, to make it easier for the reviewer to understand why / how they're being introduced. A good rule of thumb (not a hard rule) is whether you have unit tests for it.

Accepting to unblock though

This revision is now accepted and ready to land.Apr 12 2023, 5:48 PM

In general I think small utility functions like this should be introduced alongside their callsite, to make it easier for the reviewer to understand why / how they're being introduced. A good rule of thumb (not a hard rule) is whether you have unit tests for it.

Will keep this rule of thumb in mind moving forward