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

Add option to ignore all virtual members #58

Merged
merged 7 commits into from
Dec 1, 2022
Merged

Conversation

Cyaminthe
Copy link
Contributor

Similarly to IncludeCompilerGeneratedMembers, adds optional metadata IncludeVirtualMembers.
When it is set to false, all virtual members in the assembly are exempt from publicizing (unless specified explicitly).
The default value is true, to not alter previously expected behaviour.

Copy link
Owner

@krafs krafs left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! Contributors are always welcome :)

Besides a couple of minor comments, I'd still like some kind of test for this. I realize that the existing tests are bulky and hard to read, but they are better than nothing. If you can add a similar test for IncludeVirtualMembers (e.g. adapting the test for CompilerGeneratedMembers) I'd feel better approving the PR.

Again - thanks for helping out!

README.md Outdated Show resolved Hide resolved
src/Publicizer/Publicizer.csproj Outdated Show resolved Hide resolved
@Cyaminthe
Copy link
Contributor Author

Besides a couple of minor comments, I'd still like some kind of test for this. I realize that the existing tests are bulky and hard to read, but they are better than nothing. If you can add a similar test for IncludeVirtualMembers (e.g. adapting the test for CompilerGeneratedMembers) I'd feel better approving the PR.

Sure thing. Added a test (and fixed the minor comments).

@Cyaminthe Cyaminthe requested a review from krafs November 30, 2022 20:18
@krafs krafs self-assigned this Dec 1, 2022
@krafs krafs added the enhancement New feature or request label Dec 1, 2022
@krafs krafs merged commit 5edad7c into krafs:main Dec 1, 2022
@krafs
Copy link
Owner

krafs commented Dec 1, 2022

Thanks! I'll try to release soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants