Page MenuHomePhabricator

[lib] introduce getRandomDefaultEmojiAvatar function
ClosedPublic

Authored by ginsu on Apr 12 2023, 11:49 AM.
Tags
None
Referenced Files
F1882801: D7401.id25072.diff
Tue, May 28, 5:07 AM
F1882800: D7401.id25069.diff
Tue, May 28, 5:07 AM
F1882799: D7401.id25050.diff
Tue, May 28, 5:07 AM
F1882770: D7401.id.diff
Tue, May 28, 5:07 AM
F1882735: D7401.diff
Tue, May 28, 5:02 AM
Unknown Object (File)
Sun, May 26, 8:58 AM
Unknown Object (File)
Sun, May 26, 8:58 AM
Unknown Object (File)
Fri, May 24, 5:49 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