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

chore: enum support for new API #7110

Merged
merged 3 commits into from
Feb 14, 2024
Merged

Conversation

benmccann
Copy link
Contributor

@benmccann benmccann commented Feb 14, 2024

@jrasm91 has been doing a great job migrating the API over. One of the TODOs from #7103 was enums. I got that setup and working in this PR. I only migrated a single enum as a demonstration to avoid creating merge conflicts with his outstanding PR

Looks like this is causing a bunch of type errors, so there are a bunch of places we'll need to update imports from @api to @immich/sdk. It's probably best to wait for #7103 before doing that. Feel free to take this over as I have a busy day tomorrow or else I'll get back to it later in the week

@benmccann benmccann marked this pull request as draft February 14, 2024 05:26
Copy link
Contributor

@jrasm91 jrasm91 left a comment

Choose a reason for hiding this comment

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

I've converted all imports and endpoints to use the new enum types. The only things left are a few apis that use progress events and blob responses.

@jrasm91 jrasm91 marked this pull request as ready for review February 14, 2024 14:12
@@ -29,7 +28,8 @@
JobName.Migration,
];

function isSystemConfigJobDto(jobName: JobName): jobName is keyof SystemConfigJobDto {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not much of a Typescript expert. Could this be unkown to avoid the any?

Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't look like it:
image

@benmccann
Copy link
Contributor Author

your changes look good to me! thanks for finishing this up!!

@benmccann benmccann merged commit 87ae0be into immich-app:main Feb 14, 2024
24 checks passed
@benmccann benmccann deleted the one-enum branch February 14, 2024 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants