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

Added VB (specific/focused) SourceGenerators Sample (Try #2) #692

Merged
merged 1 commit into from
May 24, 2021

Conversation

DualBrain
Copy link
Contributor

Couldn't figure out how to "rebase" in order to fix the last submission; so I started from the beginning. This one shouldn't have any of the bin/obj folders.

Reference rest of conversation in #691

Thanks for your consideration.

@DualBrain
Copy link
Contributor Author

Forgot to tag @KathleenDollard when I originally submitted this one. Doing so now.

Base automatically changed from master to main March 5, 2021 07:22
@DualBrain
Copy link
Contributor Author

If not accepted, would it be possible to add a link to an external repo where the VB'ified versions could be found - thus removing the responsibility by the Roslyn team to maintain a VB specific set of examples. This link would, of course, have any and all disclaimers and mention that it's external of Microsoft yada, yada, yada...

I ask this as it was suggested that there isn't any reason to have a C# and VB version of the samples as VB developers could simply look at the existing C# ones to "get the gist". I somewhat disagree with this sentiment as I think many VB devs would just assume that it doesn't work with VB since there aren't any examples in it and not waste their time investigating it any further (again, lacking examples demonstrating that it does indeed work). I'm a fan of Source Generators... even if there is clearly more work to be done in some areas... but a fan nonetheless. So I'd love for members of my community (VB) to share in my excitement. I have no issue with my version of the samples being somewhere else; however, that doesn't stop people from clicking on blog posts that take them to this repo and them seeing nothing for them. So at least a link pointing them somewhere else would at least something...

I currently have them available as part of my general Samples repo at https://github.com/DualBrain/Samples/tree/master/SourceGenerators

In addition to the existing C# inspired originals, my repo also includes an example that attempts to reproduce many of the behaviors of C# Records in VB through a Source Generator; and I'm currently exploring/experimenting with another Source Generator that would work to handle Implicit Interface handling (similar to C#... where possible) that will be added to that repo.

@KathleenDollard

@jmarolf jmarolf requested review from jmarolf and chsienki May 20, 2021 00:02
@jmarolf
Copy link
Contributor

jmarolf commented May 20, 2021

Looks good @DualBrain to merge this you will need to add the new project to the Samples.sln solution here. Once CI passes with those changes I think we will be good to go.

@jmarolf
Copy link
Contributor

jmarolf commented May 20, 2021

If not accepted, would it be possible to add a link to an external repo where the VB'ified versions could be found - thus removing the responsibility by the Roslyn team to maintain a VB specific set of examples. This link would, of course, have any and all disclaimers and mention that it's external of Microsoft yada, yada, yada...

This argument doesn't apply here. The Roslyn uses both C# and VB on a regular basis and we can certainly understand and own these samples.

@chsienki
Copy link
Member

LGTM modulo updating the SLN.

@jmarolf
Copy link
Contributor

jmarolf commented May 24, 2021

alright, I am going to merge this and fix up the sln file in a different PR

@jmarolf jmarolf merged commit 3b02486 into dotnet:main May 24, 2021
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.

3 participants