Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add update metadata route #12

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Add update metadata route #12

wants to merge 2 commits into from

Conversation

Giska
Copy link
Contributor

@Giska Giska commented Dec 6, 2023

No description provided.

@Giska Giska requested a review from yma December 6, 2023 13:33
Comment on lines +549 to 560
export enum TextedVideoType {
Full = "Full",
Partial = "Partial",
InsertOnly = "InsertOnly",
}

export enum FileType {
Video = "Video",
Audio = "Audio",
Subtitle = "Subtitle",
Extra = "Extra",
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Il y a une raison d'utiliser enum plutôt que type?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

J'aime bien les enum car on peut les reutiliser si on a besoin dans le code. Contrairement aux types

src/types.ts Outdated
@@ -504,18 +504,59 @@ export interface FluffyStream {
subtitleType: string;
}

export interface Metadata {
export interface Metadata extends partialMetadata {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Si tu extends PartialMetadata alors les champs seront optionnels (?:) en plus d'être undefinable. Ça ne devrait pas être le cas avec Metadata.

Il y a moyen de réutiliser PartialMetadata en enlevant les ?: :

Suggested change
export interface Metadata extends partialMetadata {
export type Metadata = { [K in keyof partialMetadata]-?: partialMetadata[K] } & {

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

alors ton type est littéralement incompréhensible. Et pour le coup les champs de metadata avec ? sont vraiment optionnels

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Il sont nullable mais pas optionnel non? Par optionnel je veux dire que si tu oublis de mettre le field dans l'object typescript ne fera pas de warning et la valeur sera undefined (pas null).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ils sont techniquement les deux. Car flubber va te renvoyer null si l'objet n'existe pas. Mais pour le coup tu peux lui envoyer un json partiel et il est content aussi. J'ai pas hyper envie d'envoyer tout l'objet metadata. Car il est tres rarement tout utilisé

src/types.ts Outdated
broadcasterShowId: null;
}

export interface partialMetadata {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

C'est normal le "p" minuscule?

Suggested change
export interface partialMetadata {
export interface PartialMetadata {

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

C'est un oubli ça par contre

@Giska Giska requested a review from yma December 6, 2023 14:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants