Cleanup ifdefs and revise hashing algorithm by Shane32 · Pull Request #503 · Shane32/QRCoder
Conversation
Cleans up ifdefs
To the extent possible:
SYSTEM_DRAWINGis used for when System.Drawing.Common is availableNET6_0_OR_GREATERis used to applySupportedOSPlatformAttribute(maybe in future PR this is polyfilled)!NETSTANDARD1_3is used to exclude code that requires System.Drawing.Primitives
And for tests:
SYSTEM_DRAWINGis used for when System.Drawing.Common is availableTEST_XAMLis used for XAML tests!NETCOREAPP1_1is used to exclude tests for .NET Standard 1.1, and for hash checks for PNG tests
All other ifdefs are simplified and used as minimally as possible (e.g. NETFRAMEWORK vs NET35 || NET40)
Also, many tests were not performed on .NET 6, probably since the hashes don't match due to a new deflate algorithm. Worse yet, they do not appear to be deterministic, so rerunning the test can produce a different hash. I've changed the hashing algorithm to be deterministic based on the pixel values inside the Bitmap. This only works on platforms with access to Bitmap so the PNG tests still have ifdefs when running on .NET Core 1.1.
Shane32
changed the title
[WIP] Cleanup ifdefs
Cleanup ifdefs and revise hashing algorithm
I'm heading out now but if you read my comments (which I forgot to post last night), I think it will answer some of your questions. I should be able to answer any other questions by tomorrow.
I'm heading out now but if you read my comments (which I forgot to post last night), I think it will answer some of your questions. I should be able to answer any other questions by tomorrow.
Sure, will check your comments. Thanks again! :-)
Having a look onto the compatability matrix, I asked myself why e.g. PostscriptQRCode isn't compatible with .NET Standard 1.3. I checked the history/blames and saw that the exclusion ifdef intially was defined as !PCL" and then !NETSTANDARD1_1`. Since we're targeting 1.3 now, the exclusion is proably not necessary any longer. Same story for SvgQRCode.
I wish I had commented back then which feature/function was missing in PCL when I excluded it, so that I could check now if its available in .NET Standard 1.3. I guess I have to simply try out...
Having a look onto the compatability matrix, I asked myself why e.g. PostscriptQRCode isn't compatible with .NET Standard 1.3. I checked the history/blames and saw that the exclusion ifdef intially was defined as
!PCL" and then!NETSTANDARD1_1`. Since we're targeting 1.3 now, the exclusion is proably not necessary any longer. Same story for SvgQRCode.I wish I had commented back then which feature/function was missing in PCL when I excluded it, so that I could check now if its available in .NET Standard 1.3. I guess I have to simply try out...
Got it. It's System.Drawing not beeing availble for NET_STANDARD1_3. So ignore my last comment.
No; this doesn't change the exposed API.
Correction: with the exception of adding the methods that were accidentally missing from .NET 6
If you like, I can help write a test that produces a copy of the API so it is easy to tell when it change
That looks great! I've seen that you already started in #504 - thanks! :-)
Do you mind if I start to pack QRCoder for the release without waiting for 504?
Do you mind if I start to pack QRCoder for the release without waiting for 504?
Certainly! #504 won't change the compiled code.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters