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

refactor: orientation #12792

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

refactor: orientation #12792

wants to merge 1 commit into from

Conversation

jrasm91
Copy link
Contributor

@jrasm91 jrasm91 commented Sep 19, 2024

Fixes #3022

Use an enum for exif orientation, including sending it back in the asset response dtos.

@jrasm91 jrasm91 force-pushed the refactor/exif-orientation branch from ec48359 to 8af7a9c Compare September 19, 2024 16:06
@danieldietzler
Copy link
Member

Might also fix #10208?

@@ -25,8 +26,8 @@ export class ExifEntity {
@Column({ type: 'bigint', nullable: true })
fileSizeInByte!: number | null;

@Column({ type: 'varchar', nullable: true })
orientation!: string | null;
@Column({ type: 'enum', enum: Orientation, nullable: true })
Copy link
Contributor

@mertalev mertalev Oct 24, 2024

Choose a reason for hiding this comment

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

For this case, the keys are basically already enum values. It'd be nice to make this a "char" (not char) and use a check constraint that it's between 1-8. It's a 1 byte integer instead of a 4 byte float and doesn't have the string vs number ambiguity that an enum would have.

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.

[TECH-DEBT] Change orientation field type in ExifEntity and ExifResponseDto
3 participants