Page MenuHomePhabricator

[lib] introduce getRandomDefaultEmojiAvatar function
ClosedPublic

Authored by ginsu on Apr 12 2023, 11:49 AM.
Tags
None
Referenced Files
F5338109: D7401.id.diff
Wed, Apr 9, 1:05 AM
F5336146: D7401.diff
Wed, Apr 9, 12:06 AM
F5335241: D7401.id25050.diff
Tue, Apr 8, 11:15 PM
F5333878: D7401.id25072.diff
Tue, Apr 8, 10:38 PM
Unknown Object (File)
Tue, Apr 8, 8:39 PM
Unknown Object (File)
Sat, Mar 15, 2:52 PM
Unknown Object (File)
Mar 4 2025, 2:52 AM
Unknown Object (File)
Mar 4 2025, 2:52 AM
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