Skip to content

Fix AddClasses method to take only public classes not private#238

Merged
khellang merged 1 commit intokhellang:masterfrom
AntonyNET:master
Dec 23, 2024
Merged

Fix AddClasses method to take only public classes not private#238
khellang merged 1 commit intokhellang:masterfrom
AntonyNET:master

Conversation

@AntonyNET
Copy link
Copy Markdown
Contributor

Documentation of method IImplementationTypeSelector.AddClasses(Action action) says that "Adds all public, non-abstract classes from the selected assemblies that" but in implementation this method takes private classes too:
AddClasses(action, publicOnly: false).

Sync code with documentation.

@khellang
Copy link
Copy Markdown
Owner

khellang commented Nov 9, 2024

Hmm. It's really unfortunate that the different overloads have different behaviors. No clue how it ended up that way. But this is a pretty big breaking change, as code that previously resolved fine, would now start throwing. I need to consider this for a bit 😅

@khellang khellang merged commit 0b1d216 into khellang:master Dec 23, 2024
@khellang
Copy link
Copy Markdown
Owner

khellang commented Dec 23, 2024

Let's see how it works out 😅 Thanks @AntonyNET!

@alfeg
Copy link
Copy Markdown

alfeg commented Dec 24, 2024

Ouch. Quite a breaking one.
Hopefuly fast to fix.

@bensmind
Copy link
Copy Markdown

Should have been a change to the documentation to match code rather than the code changing to match documentation. This is a very large breaking change that got included on a minor release.

@khellang
Copy link
Copy Markdown
Owner

Yeah, if it turns out this will break a lot of people, we might want to release it as a major version instead

@khellang
Copy link
Copy Markdown
Owner

Hopefuly fast to fix.

It's VERY fast to fix; just pass publicOnly: false explicitly.

@sailro
Copy link
Copy Markdown

sailro commented Jan 11, 2025

It is very fast to fix indeed, but broke everything on our side unfortunately (before we figure out the root cause).

And fully agree with:
#238 (comment)

@khellang
Copy link
Copy Markdown
Owner

See #244

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants