-
Notifications
You must be signed in to change notification settings - Fork 2
Conversation
Deploying with
|
Latest commit: |
2b632d6
|
Status: | ✅ Deploy successful! |
Preview URL: | https://2c4c8768.cast-iu.pages.dev |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please rename the supabase
directory to api
?
@alkaitagi I think the GitHub usernames for the reviewer lottery are case-sensitive. Max was assigned two reviewers, I suppose it is because he's not recognized as |
Btw, I don't think that "implements" is a valid keyword to close issues in GitHub. I think the options are |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Final set of comments
src/lib/shared/api/adapters.ts
Outdated
id: episode.id, | ||
title: episode.title, | ||
duration: episode.duration, | ||
audioUrl: episode.recording_object_id, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question: is this really going to be a URL? I'm concerned it might just be an ID
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Working on it with @khaledismaeel. There's an issue with Supabase we need to solve first
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Supabase storage is bad in millions of ways, so I added a column audio_url
to the Episodes table. Yeah, we won't be able to upload the music now, only add a link
src/lib/shared/api/episodes.ts
Outdated
import { transformEpisodeRequest } from './adapters'; | ||
import supabaseClient from './supabase'; | ||
|
||
export const episodeRead = async (): Promise<Episode[]> => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue: episodeRead
sounds ambiguous to me. I think of "read" and "get" as synonyms in terms of APIs.
suggestion: perhaps listEpisodes
could work better?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed to episodesList
, so, when someone will import from api directory, it'll be import { episidesList } from '$lib/shared/api';
. Since people first think about an entity, and then about the method, the names should be easy to find.
…to feature/supabase-integration # Conflicts: # src/lib/shared/api/index.ts
Implements #39
Since now, the use of the app requires adding Environmental variables. To get them, go into the Supabase dashboard -> settings -> API, and copy 'service_role'-'secret' token into the
VITE_SUPABASE_KEY
, and URL variable into theVITE_SUPABASE_URL
.