Skip to content

Beta7#993

Merged
JimBobSquarePants merged 5 commits intomasterfrom
js/update-submodule
Sep 4, 2019
Merged

Beta7#993
JimBobSquarePants merged 5 commits intomasterfrom
js/update-submodule

Conversation

@JimBobSquarePants
Copy link
Copy Markdown
Member

@JimBobSquarePants JimBobSquarePants commented Sep 1, 2019

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 matches 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

I haven't enabled StyleCop for the tests yet as it's a days worth of work to cleanup all the issues there and it can wait until after the release. If someone can please review the linked PRs and see if they are in a mergeable state. The bug fix must definitely go in, the enhancement can possibly wait.

@codecov
Copy link
Copy Markdown

codecov bot commented Sep 1, 2019

Codecov Report

Merging #993 into master will increase coverage by 0.1%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #993     +/-   ##
=========================================
+ Coverage   89.67%   89.77%   +0.1%     
=========================================
  Files        1100     1098      -2     
  Lines       48926    48812    -114     
  Branches     3447     3433     -14     
=========================================
- Hits        43872    43821     -51     
+ Misses       4297     4285     -12     
+ Partials      757      706     -51
Impacted Files Coverage Δ
src/ImageSharp/Memory/BufferArea{T}.cs 97.05% <ø> (ø) ⬆️
...ssing/Processors/Transforms/Resize/ResizeHelper.cs 45.61% <0%> (ø) ⬆️
...files/ICC/DataWriter/IccDataWriter.TagDataEntry.cs 77.14% <0%> (ø) ⬆️
...ils/ReferenceImplementations.StandardIntegerDCT.cs 93.68% <0%> (ø) ⬆️
src/ImageSharp/Formats/Bmp/BmpDecoderCore.cs 94.36% <0%> (+0.17%) ⬆️
src/ImageSharp/Formats/Png/PngDecoderCore.cs 85.65% <0%> (+0.21%) ⬆️
...files/ICC/DataReader/IccDataReader.TagDataEntry.cs 85.79% <0%> (+0.28%) ⬆️
...harp.Tests/Drawing/FillLinearGradientBrushTests.cs 99.56% <0%> (+0.43%) ⬆️
src/ImageSharp/Formats/Png/PngEncoderCore.cs 96.91% <0%> (+0.47%) ⬆️
...ssors/Quantization/OctreeFrameQuantizer{TPixel}.cs 98.07% <0%> (+0.64%) ⬆️
... and 20 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5c06211...7c94922. Read the comment docs.

Copy link
Copy Markdown
Member

@antonfirsov antonfirsov left a comment

Choose a reason for hiding this comment

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

A few concerns to address before we merge:

  • There still might be a bug in Adaptive Histogram Equalization (see my comment). Do we want to wait until it gets fixed? Or just merge #984 as-is and open an issue?
  • There are breaking changes in the packages we reference. We may want to enforce that a package update on SixLabors.ImageSharp and SixLabors.ImageSharp.Drawing also pulls the latest beta versions of Core, Fonts, and Shapes. I think we may want to specify a minimum version in <PackageReference ... />, but I'm not familiar with this stuff.

</PropertyGroup>

<!-- TODO: Include .NETSTANDARD2.1 when released-->
<PropertyGroup Condition=" $(TargetFramework.StartsWith('netcoreapp2')) ">
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.

👍 cool stuff

@antonfirsov
Copy link
Copy Markdown
Member

antonfirsov commented Sep 1, 2019

image

Ooops I did this by accident, we can revert it, if we want.

@JimBobSquarePants
Copy link
Copy Markdown
Member Author

@antonfirsov the minimum version for Core etc is specified already in the targets file. Let’s ship the equaliser as-is and raise a new ticket.

<PackageReference Update="SixLabors.Fonts" Version="1.0.0-beta0008" />
<PackageReference Update="SixLabors.Shapes" Version="1.0.0-beta0008" />
<PackageReference Update="SixLabors.Shapes.Text" Version="1.0.0-beta0007" />
<PackageReference Update="SixLabors.Core" Version="1.0.0-beta0008" />
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@antonfirsov see here for min versions

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.

Thanks, totally missed this.

@Rwing
Copy link
Copy Markdown

Rwing commented Sep 4, 2019

Is there release schedule? I can't wait to use the new version

@JimBobSquarePants
Copy link
Copy Markdown
Member Author

@Rwing Most likely within the nest 12 hours.

@JimBobSquarePants JimBobSquarePants changed the title WIP: Beta7 Beta7 Sep 4, 2019
@JimBobSquarePants JimBobSquarePants merged commit be5c99f into master Sep 4, 2019
@JimBobSquarePants JimBobSquarePants deleted the js/update-submodule branch September 4, 2019 06:57
antonfirsov pushed a commit to antonfirsov/ImageSharp that referenced this pull request Nov 11, 2019
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.

3 participants