Skip to content

Remove #nullable disable from ExifProfile - #2320

Merged
JimBobSquarePants merged 9 commits into
SixLabors:mainfrom
stefannikolei:stefannikolei/nullable/exifprofile
Jan 26, 2023
Merged

JimBobSquarePants merged 9 commits into
SixLabors:mainfrom
stefannikolei:stefannikolei/nullable/exifprofile

Conversation

@stefannikolei

@stefannikolei stefannikolei commented Jan 17, 2023 •

Copy link
Copy Markdown
Contributor

Prerequisites

  • I have written a descriptive pull-request title
  • I have verified that there are no overlapping pull-requests open
  • I have verified that I am following the existing coding patterns and practice as demonstrated in the repository. These follow strict Stylecop rules 👮.
  • I have provided test coverage for my change (where applicable)

Description

Resolves the nullable issues in the ExifProfile space #2231

@JimBobSquarePants Do you think we can tackle it like that, or do you have other opinions how we could/should do it?

I had a bit of a struggle with the Generic TValueType. I tried to introduce were TValueType : struct. But there are cases where TValueType = string --> zonk

I added an InvalidOperationException to 'private static Encoding JIS0208Encoding' to not expose Encoding? because that will never happen. What do you think about this? Wanna go this way or should I add an ! here to dismiss the warning

Comment thread src/ImageSharp/Metadata/Profiles/Exif/ExifProfile.cs Outdated
public override ExifDataType DataType => ExifDataType.Byte;

protected override string StringValue => this.Value;
protected override string? StringValue => this.Value;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

After looking through the changes, I think I wasn't consistent with this override. Should I add the ? in all ExifValue types?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ok compiler tells me that it can be removed in the other places

@stefannikolei
stefannikolei marked this pull request as ready for review January 18, 2023 07:45
Comment thread src/ImageSharp/Metadata/Profiles/Exif/ExifEncodedStringHelpers.cs Outdated
Comment thread src/ImageSharp/Metadata/Profiles/Exif/ExifProfile.cs Outdated
@JimBobSquarePants JimBobSquarePants mentioned this pull request Jan 23, 2023
4 tasks done
int tileLength = (int)tags.GetValue(ExifTag.TileLength).Value;
if (!tags.TryGetValue(ExifTag.TileWidth, out IExifValue<Number> valueWidth))
{
ArgumentNullException.ThrowIfNull(valueWidth);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This should be an InvalidImageContentException

Comment thread src/ImageSharp/Formats/Tiff/TiffDecoderCore.cs Outdated
Comment thread src/ImageSharp/Formats/Tiff/TiffEncoderCore.cs
Comment thread src/ImageSharp/Metadata/Profiles/Exif/ExifProfile.cs
Comment thread src/ImageSharp/Metadata/Profiles/Exif/ExifReader.cs
Comment thread src/ImageSharp/Metadata/Profiles/Exif/ExifTagDescriptionAttribute.cs Outdated
Comment thread src/ImageSharp/Metadata/Profiles/Exif/ExifWriter.cs Outdated
Comment thread src/ImageSharp/Metadata/Profiles/Exif/ExifWriter.cs
stefannikolei and others added 4 commits January 23, 2023 20:36
* Throw InvalidImageContentException
* Change GetDesctiption to TryGetDescription
* Do not throw when creating exifvalue and adding it to ifdValues

@JimBobSquarePants JimBobSquarePants left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. Thanks! 👍

@JimBobSquarePants JimBobSquarePants added API codequality breaking Signifies a binary breaking change. labels Jan 26, 2023
@JimBobSquarePants JimBobSquarePants added this to the 3.0.0 milestone Jan 26, 2023
@JimBobSquarePants
JimBobSquarePants merged commit 22840b3 into SixLabors:main Jan 26, 2023
@stefannikolei
stefannikolei deleted the stefannikolei/nullable/exifprofile branch January 26, 2023 07:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

API breaking Signifies a binary breaking change. codequality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants