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

Replace custom attribute shims with PolySharp #397

Merged
merged 2 commits into from
Aug 10, 2023

Conversation

lahma
Copy link
Collaborator

@lahma lahma commented Aug 10, 2023

While the original reasoning to have own versions of the attribute shims due to concerns about supply chain attacks, I would argue that we can trust in the author of PolySharp who is a well recognized Microsoft employee. This would also gives us many other missing bits if we ever needed them. Thoughts?

@lahma lahma requested a review from adams85 August 10, 2023 07:52
Copy link
Collaborator

@adams85 adams85 left a comment

Choose a reason for hiding this comment

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

As long as it's a dev-time dependency, I don't have too strong concerns for adding a 3rd party dependency. The lib and its author look solid, so I have nothing against it.

I just suggest we are explicit about the included polyfills, that is, let's include only those which are really needed. (Sorry for meddling with your PR, was quicker this way for me. Just drop my commit if you disagree.)

@lahma
Copy link
Collaborator Author

lahma commented Aug 10, 2023

Thanks, explicit looks good!

@lahma lahma merged commit 6b5fbf1 into sebastienros:main Aug 10, 2023
2 checks passed
@lahma lahma deleted the polysharp branch August 10, 2023 12:30
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.

2 participants