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

Feature/net6 workaround #102

Merged
merged 3 commits into from
Apr 13, 2022

Conversation

TristanBoland
Copy link
Contributor

It seems the QRCode Class is no longer available in QRCoder for .NET6.
Throws an "Could not load type 'QRCoder.QRCode' from assembly 'QRCoder, Version=1.4.3.0" exception currently.
I needed this library in one of my projects.
So i decided to publish my workaround to here. Hopefully it is of use to some

It seems the QRCode Class is no longer available in QRCoder for .NET6. I needed this library in one of my projects. So i decided to publish my workaround to here. Hopefully it is of use to some
@ahwm
Copy link
Collaborator

ahwm commented Apr 13, 2022

@flytzen thoughts? I've been seeing more of these requests, not sure how we want to proceed.

@flytzen
Copy link
Collaborator

flytzen commented Apr 13, 2022

@flytzen thoughts? I've been seeing more of these requests, not sure how we want to proceed.

Well, if this works - which it looks like it does - then we should pull it in for now. We do need to test it on Linux, though. Even if it is "windows only", we can still do it as a short-term thing. I'll run the test suite on Linux.
But if codebude/QRCoder#315 isn't resolved soon, then I guess we will have to switch to a different QR coder library.

Relates to #100 and #98 and #94.
If we do switch to another lib then we probably want/need to drop legacy support (see #89) and only support netstandard2.0 up and bump the main version.

@TristanBoland Thank you!!

@flytzen
Copy link
Collaborator

flytzen commented Apr 13, 2022

@ahwm The tests are all passing on Linux as well, so I think we should merge this. I do want to just check a couple of things before we merge :)

Copy link
Collaborator

@flytzen flytzen left a comment

Choose a reason for hiding this comment

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

This is really great, thank you.
Minor suggestion to avoid duplication.
Slightly more important about adding the using statements to avoid resource leaking in high volume scenarios :)

Google.Authenticator/TwoFactorAuthenticator.cs Outdated Show resolved Hide resolved
Co-authored-by: Frans Lytzen <flytzen@neworbit.co.uk>
@TristanBoland
Copy link
Contributor Author

I accepted your changes, glad i could help! Was intended as a quick fix so i could go on with my Task, but glad it was of use

Copy link
Collaborator

@flytzen flytzen left a comment

Choose a reason for hiding this comment

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

@ahwm I'm approving this.
I'll do a separate PR with a new version number and a minor fix for a new warning in .Net 6.0.

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.

None yet

3 participants