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

ISSUE #2385 - Implementing try get pattern to TryFindFormatByFileExtension(string extension, [NotNullWhen(true)] out IImageFormat? format) #2386

Merged
merged 3 commits into from
Mar 6, 2023

Conversation

Ollie-Ave
Copy link
Contributor

@Ollie-Ave Ollie-Ave commented Mar 6, 2023

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

An issue was raised recently (#2385) Implementing try get pattern to public bool TryFindFormatByFileExtension(string extension, [NotNullWhen(true)] out IImageFormat? format). This PR simply removes Guard.NotNullOrWhiteSpace(extension, nameof(extension)); on line 105 in amendment to this.

I have also added a check for null or whitespace before the check for the presence of a '.' character to prevent out of range exceptions.

if (!string.IsNullOrWhiteSpace(extension) && extension[0] == '.')
{
    extension = extension[1..];
}

I have not edited any other code and there were no extra Unit Test failures after this change was made (1 unit test was failing before any changes were made to the branch)

@CLAassistant
Copy link

CLAassistant commented Mar 6, 2023

CLA assistant check
All committers have signed the CLA.

@JimBobSquarePants
Copy link
Member

@Ollie-Ave

Thanks for this. That's a neat fix 👍

Was that locally failing test the memory one? The test explorer in VS doesn't respect the conditional value on the query. It works via dotnet test.

@bjkippax
Copy link

bjkippax commented Mar 6, 2023

@JimBobSquarePants no failures as far as I am aware but I will let @Ollie-Ave follow-up with you.

What's your release schedule?

@Ollie-Ave
Copy link
Contributor Author

@JimBobSquarePants
It was, the test name was GC_Collect_OnHighLoad_TrimsEntirePool. After running a dotnet test, all tests are passing. Good to know for future contributions.

Thanks for the speedy response with this issue as a whole.

@JimBobSquarePants
Copy link
Member

What's your release schedule?

You'll be able to access this release now on our nightly feed.

@Ollie-Ave
Copy link
Contributor Author

What's your release schedule?

You'll be able to access this release now on our nightly feed.

When will this be available via NuGet?

@JimBobSquarePants
Copy link
Member

I don’t know. We only released v3.0.0 a few days ago so will likely capture a few more issues first. This is non-critical so does not require immediate release

@Ollie-Ave
Copy link
Contributor Author

Fair enough, thanks for your help!

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