refactor(cli): organize files, simplify types, use @immich/sdk #6747
Conversation
jrasm91
commented
Jan 30, 2024
•
jrasm91
commented
| Before | After |
|---|---|
 |
 |
Deploying with
|
| Latest commit: |
0330dba
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://ef340894.immich.pages.dev |
| Branch Preview URL: | https://refactor-cli-1.immich.pages.dev |
jrasm91
force-pushed
the
refactor/cli-1
branch
from
e767874 to
0330dba
Compare
| @@ -1,31 +1,40 @@ | |||
| import fs from 'node:fs'; | |||
| import yaml from 'yaml'; | |||
| import { existsSync } from 'fs'; | |||
There was a problem hiding this comment.
I probably wouldn't remove the node: prefix here
| import { existsSync } from 'fs'; | |
| import { existsSync } from 'node:fs'; |
There was a problem hiding this comment.
True, true. Do you know if there is a linting rule or some way to prefer the prefix? The code base is quite inconsistent.
There was a problem hiding this comment.
yeah, there's unicorn/prefer-node-protocol: https://github.com/sindresorhus/eslint-plugin-unicorn/blob/main/docs/rules/prefer-node-protocol.md
There was a problem hiding this comment.
I created #6751 as a proof of concept for this (and most other unicorn rules)
| loginCredentialDto: { email: 'cli@immich.app', password: 'password' }, | ||
| }); | ||
| const { data: apiKey } = await api.keyApi.createApiKey( | ||
| { aPIKeyCreateDto: { name: 'CLI Test' } }, |
There was a problem hiding this comment.
the capitalization looks off here
There was a problem hiding this comment.
Haha yeah. This is the auto generated code from open API generator. I think if we changed the dto on the server from APIKey to ApiKey it would generate more how you would expect it to.
There was a problem hiding this comment.
Based on this SO post, it seems style guidelines for acronyms lean toward using the normal casing instead of all-caps. So XmlHttpRequest, not XMLHTTPRequest. Maybe we should adopt that for consistency.
| if (!this.fileCreatedAt) throw new Error('File created at not set'); | ||
| if (!this.fileModifiedAt) throw new Error('File modified at not set'); | ||
|
|
||
| // TODO: doesn't xmp replace the file extension? Will need investigation |
There was a problem hiding this comment.
Normally you get a .xmp after the original extension, so foo.jpg.xmp
There was a problem hiding this comment.
This is actually not standardized! I've used a bunch of photo tools in my workflow and some use foo.xmp and some use foo.jpg.xmp. This is something on my backlog to fix across all of Immich, currently we only support one format.
