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

Make QRCoder trimmable and add trimming tests #539

Merged
merged 4 commits into from
May 28, 2024

Conversation

Shane32
Copy link
Contributor

@Shane32 Shane32 commented May 25, 2024

Summary

This marks the QRCoder project as trimmable and fixes existing trim warnings within the project.

Changes / notes

  1. The RussianPaymentOrder payload generator uses reflection to add all properties to a list. Users cannot provide their own derived class, so the property list is known. As such, reflection is not even necessary. However, by simply referencing the exact type in the code, trimming analysis will know to preserve all the relevant fields.
  2. The SVG generator uses reflection to pull an associated string value (mime type) from an enum. Similar to above, the type of the enum examined is always known and cannot change, so reflection is unnecessary. In this instance I changed the code to use a switch case to return the proper mime type, eliminating the use of reflection. The code that used reflection was publicly accessible and remains (marked as obsolete) to maintain binary backwards compatibility. It is quite unlikely any users ever referenced these members, of course.

Test plan

Pursuant to MS docs, a new project has been added, that when published, performs trim analysis on QRCoder. Trim analysis has been configured to fail the publish. CI has been configured to 'publish' this upon running the tests.

Closing issues

Closes #537

@Shane32
Copy link
Contributor Author

Shane32 commented May 25, 2024

@csturm83 Didn't realize you were going to work on this! Apparently you pushed a PR just around the same time I did!

@Shane32
Copy link
Contributor Author

Shane32 commented May 25, 2024

To see how the trimmable tests work, look at the first CI run in this PR. You'll see something like this:

image

@codebude
Copy link
Owner

Well done! Had to find a minute to learn about trimming and its use cases, to then review your PR. Looks good to me. Thanks!

@codebude codebude merged commit 8bfcf40 into codebude:master May 28, 2024
3 checks passed
@Shane32 Shane32 deleted the trimmable branch May 28, 2024 20:17
@codebude
Copy link
Owner

Updated the release notes: https://github.com/codebude/QRCoder/wiki/Release-Notes/_compare/f245943d08e6efb9810f10313a80150b168b506f...ed3367a8fdde6c8ebbfa2d07b38fd2033b001d72

Let me know if I missed anything (or if you see room for improvements).

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.

[Feature] Trimmability
2 participants