-
Notifications
You must be signed in to change notification settings - Fork 0
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
Init storage shipper #2
base: main
Are you sure you want to change the base?
Conversation
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.
Sprawdziłem pod tym kątem co prosiłeś narazie bez zagłębiania się narazie w to co robią poszczególne funkcje. Ogólnie wygląda ok, kilka drobnych uwaga dodałem
@@ -0,0 +1,17 @@ | |||
export interface Artifact { |
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.
jeśli chodzi o typy to zazwyczaj robimy tak że:
- Jak projekt jest mały to w src/ jest jeden plik types.ts i tam są wszystkie typy/interfejsy
- Jak projekt jest większy to w scr/ robimy katalog types/ i tam wrzucamy osobne pliki tak jak Ty zrobiłeś
data: ArrayBuffer | null | ||
} | ||
|
||
export function sanitizedParentDir (artifact: Artifact): string | null | undefined { |
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.
tą funkcje proponuje przenieść do src/utils/artifact.ts
export * from './StorageShipper' | ||
export * from './StorageUploader' | ||
|
||
export * from './artifact/Artifact' |
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.
w tym pliku powinny być eksporty tylko tych funkcji, które użytkownik bibloteki może używać, bez eksportu typów (typy powinny sie przy buildzie wygenerować automatycznie jako pliki xyz.d.ts
export interface Deploy { | ||
name: string | ||
include: string | ||
headers?: object |
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.
jak chcesz typ dla dowolnego objektu to lepiej użyć Record<string, unknown> (object to może być wszystko prawie, nawet null)
@@ -0,0 +1,63 @@ | |||
import type JSZip from 'jszip' | |||
import { minimatch } from 'minimatch/dist/mjs' |
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.
racze sporadycznie jest potrzeba importowania z dist/, jeżeli zrobiłeś tak bo ts rzucał błędy to raczej kwestia braku typów, wtedy jesli biblioteka nie jest w TS to warto poszukać typów do niej -> https://www.npmjs.com/package/@types/minimatch
@@ -0,0 +1,23 @@ | |||
import * as schema from '../../../../schema/manifest-1.0.json' | |||
import Ajv2020 from 'ajv/dist/2020' |
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.
tu podobnie do sprwadzenia czy są typy dostępne
No description provided.