Page MenuHomePhabricator

[lib] change fetchFarcasterChannelByName -> fetchFarcasterChannelByID and refactor
ClosedPublic

Authored by varun on Oct 15 2024, 12:44 PM.
Tags
None
Referenced Files
F3518295: D13718.id45214.diff
Sun, Dec 22, 7:31 PM
Unknown Object (File)
Wed, Dec 18, 8:39 AM
Unknown Object (File)
Wed, Dec 18, 8:38 AM
Unknown Object (File)
Wed, Dec 18, 8:36 AM
Unknown Object (File)
Thu, Nov 28, 9:12 PM
Unknown Object (File)
Thu, Nov 28, 6:05 PM
Unknown Object (File)
Thu, Nov 28, 6:03 PM
Unknown Object (File)
Thu, Nov 28, 5:59 PM
Subscribers

Details

Summary

realized we're conflating channel name and ID in our code. this change cleans things up a little and makes sure we're querying by ID

Depends on D13695

Test Plan

called fetchFarcasterChannelByID with a channel name ("Crypto Fantasy League") and didn't get back a response. called it again with a channel ID ("fantasyisland") and got the right response

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

varun published this revision for review.Oct 15 2024, 12:46 PM

I see D13717 was landed already but I think this diff is still an improvement. Will rebase and wait for CI before landing

lib/utils/neynar-client.js
152

Even though we only call this function from fetchFarcasterChannelByID, I think it's a useful change. we may want to query by name in the future and both types of queries use the same underlying neynar api

ashoat requested changes to this revision.Oct 15 2024, 2:00 PM

Glad you independently found the id/name bug!

I don't think we need support for fetching by name right now. Can you simplify this to just a rename?

lib/utils/neynar-client.js
152

I think this is overengineering. I don't think it's worth having this complexity in the codebase for a hypothetical future use case. We can always introduce it later when we determine that use case is needed. In general, this approach of building for hypothetical cases hurts readability and bloats the codebase, increasing the amount of maintenance required

This revision now requires changes to proceed.Oct 15 2024, 2:00 PM
This revision is now accepted and ready to land.Oct 16 2024, 9:13 PM