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

Separate UnionAttribute into its own project #204

Merged
merged 9 commits into from
Jan 4, 2024

Conversation

hwoodiwiss
Copy link
Contributor

@hwoodiwiss hwoodiwiss commented Jan 3, 2024

fixes #147

  • Splits the library into 2 projects, the attribute and the generator
  • Moves common project items to the Directory.Build.props
  • Updates Benchmarks to remove covering code that is no longer generated
  • Updates tests to work with new project layout
  • Adds SDK reference to PackAsAnalyzer
  • Updates namespaces

Testing

To test this locally, use the following steps:

  • Create a folder in the project root called .packages
  • Create a file in the project root named NuGet.Config with the following contents:
<configuration>
  <packageSources>
    <add key="local" value="./.packages" />
  </packageSources>
</configuration>
  • Open a command window in the project root and run dotnet build -c Release --no-restore
  • Copy the output nupkg file from src\Dunet\bin\Release to the .packages folder
  • Update integration tests to use Dunet 1.11.0, they should still pass, and control clicking on the attribute should (IDE dependent) take you to the decompiled attribute source.

- Splits the library into 2 projects, the attribute and the generator
- Moves common project items to the Directory.Build.props
- Updates Benchmarks to remove covering code that is no longer generated
- Updates tests to work with new project layout
- Adds SDK reference to PackAsAnalyzer
- Updates namespaces
Directory.Build.props Outdated Show resolved Hide resolved
@domn1995
Copy link
Owner

domn1995 commented Jan 3, 2024

Am I correct in my understanding that consumers will continue to install just the Dunet package as they are today but will get the generator through Dunet.Generator transitively?

@@ -22,7 +22,8 @@
</ItemGroup>

<ItemGroup>
<ProjectReference Include="..\src\Dunet.csproj" />
<ProjectReference Include="..\src\Dunet\Dunet.csproj" />
<ProjectReference Include="..\src\Dunet.Generator\Dunet.Generator.csproj" />
Copy link
Owner

Choose a reason for hiding this comment

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

Do we need this reference if Dunet.csproj references it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, for the test project we do, as Dunet.csproj doesn't forward the reference to Dunet.Generator.csproj, the same is true with the Benchmark project

Directory.Build.props Outdated Show resolved Hide resolved
@domn1995
Copy link
Owner

domn1995 commented Jan 3, 2024

Just finished testing this out locally as per the PR instructions. Dude, this is awesome! Thanks so much for working on this! I've left a few comments, but overall, it's looking really good!

@hwoodiwiss
Copy link
Contributor Author

Am I correct in my understanding that consumers will continue to install just the Dunet package as they are today but will get the generator through Dunet.Generator transitively?

Yeah, you'd still be able to just dotnet add package dunet, and both the analyzer and the attribute assembly will be included.

@hwoodiwiss hwoodiwiss marked this pull request as ready for review January 3, 2024 10:20
@hwoodiwiss
Copy link
Contributor Author

Hmm, I'll take a look at this properly tonight (GMT)

@hwoodiwiss
Copy link
Contributor Author

Ah, fixed it (I think).
It worked on my machine because I dev on Windows

@domn1995 domn1995 merged commit f6a1359 into domn1995:main Jan 4, 2024
1 check passed
@hwoodiwiss hwoodiwiss deleted the hwoodiwiss-split-project branch January 4, 2024 07:12
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.

Separate Attributes into another project
2 participants