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

Added more customization options for generating different types of im… #1305

Merged
merged 1 commit into from
Jul 16, 2024

Conversation

bodiam
Copy link
Contributor

@bodiam bodiam commented Jul 15, 2024

…ages with different dimensions.

Copy link

codecov bot commented Jul 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.70%. Comparing base (c7846a4) to head (eae5243).

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #1305      +/-   ##
============================================
- Coverage     91.76%   91.70%   -0.07%     
- Complexity     3081     3082       +1     
============================================
  Files           310      310              
  Lines          6038     6078      +40     
  Branches        628      631       +3     
============================================
+ Hits           5541     5574      +33     
- Misses          341      345       +4     
- Partials        156      159       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Comment on lines +32 to +37
BMP("image/bmp"),
GIF("image/gif"),
JPEG("image/jpeg"),
PNG("image/png"),
SVG("image/svg+xml"),
TIFF("image/tiff");
Copy link
Collaborator

@snuyanzin snuyanzin Jul 16, 2024

Choose a reason for hiding this comment

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

do we really need image/ prefix for every item?
May be just add it once in get method or in constructor of enum?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, no, we wouldn't really need it, but the field is called mimetype, which is why you have of these duplication, but without the image/ part it wouldn't be a mimetype, it would be..... ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess if you replace constructor with

 ImageType(String type) {
      this.mimeType = "images/" + type;
 }     

the field would be still mimeType however less duplication

}
}

public record Base64ImageRuleConfig(ImageType imageType, int width, int height) { }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Will it continue working if we pass Integer.MAX_VALUE ?

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 haven't tried, but most likely it wouldn't. Then again how often would you create an image of 2147483647 x 2147483647? Seems quite the picture, but I don't see a really good reason to limit it to any arbitrary value, I'd call that user error.

@bodiam bodiam merged commit 83f1187 into main Jul 16, 2024
11 of 12 checks passed
@bodiam bodiam deleted the more-image-options branch July 16, 2024 12:22
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.

4 participants