-
-
Notifications
You must be signed in to change notification settings - Fork 119
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
Separates test extensions into it's own assembly (Fixes #149) #150
Conversation
Reduces app download size for projects that are dependant on Blazor.LocalStorage by not including `bunit.core` as part of the initial download. The test extensions have been pulled out into a completely separate assembly for use by unit tests.
Builds and packages the TestExtensions nuget package
Separates test extensions into it's own package
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you revert this as it's just a whitespace change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops! Good catch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reverted.
[assembly: InternalsVisibleTo("Blazored.LocalStorage.Tests, PublicKey=" + | ||
"0024000004800000940000000602000000240000525341310004000001000100e94102d6760ebc" + | ||
"ff1970798791888ddf102ac709e19db9a312721fafca42b894652b59bada7d592a4ab62a5b7650" + | ||
"7a27720e922bc310c4f5aa75acd8ab59632c920ac41a7e9abcaf4b8bb5525a60931faccea704db" + | ||
"dcf68e1207616751447dcfec687f18854148aa66a9a09e1edc8fd0c9bd11950b4baf7d46fe3899" + | ||
"3af4add4")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did you choose to remove this attribute and move it to the csproj file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The presence of [InternalsVisibleTo]
will auto-generate a AssemblyInfo.cs file at compile time no matter where it lives. By having it appear in the SystemTextJsonSerializer.cs
file may hide that the fact that there are other arguably more important internals that we want visible to friend assemblies (e.g. IStorageProvider
). Having this in the csproj gives it a centralized location. This is my own opinion of course, and I can happily revert. 😄
<AssemblyAttribute Include="System.Runtime.CompilerServices.InternalsVisibleToAttribute"> | ||
<_Parameter1> | ||
Blazored.LocalStorage.TestExtensions, PublicKey=0024000004800000940000000602000000240000525341310004000001000100e94102d6760ebcff1970798791888ddf102ac709e19db9a312721fafca42b894652b59bada7d592a4ab62a5b76507a27720e922bc310c4f5aa75acd8ab59632c920ac41a7e9abcaf4b8bb5525a60931faccea704dbdcf68e1207616751447dcfec687f18854148aa66a9a09e1edc8fd0c9bd11950b4baf7d46fe38993af4add4 | ||
</_Parameter1> | ||
</AssemblyAttribute> | ||
</ItemGroup> | ||
|
||
<ItemGroup> | ||
<AssemblyAttribute Include="System.Runtime.CompilerServices.InternalsVisibleToAttribute"> | ||
<_Parameter1> | ||
Blazored.LocalStorage.Tests, PublicKey=0024000004800000940000000602000000240000525341310004000001000100e94102d6760ebcff1970798791888ddf102ac709e19db9a312721fafca42b894652b59bada7d592a4ab62a5b76507a27720e922bc310c4f5aa75acd8ab59632c920ac41a7e9abcaf4b8bb5525a60931faccea704dbdcf68e1207616751447dcfec687f18854148aa66a9a09e1edc8fd0c9bd11950b4baf7d46fe38993af4add4 | ||
</_Parameter1> | ||
</AssemblyAttribute> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comment about removal of attribute.
<AssemblyAttribute Include="System.Runtime.CompilerServices.InternalsVisibleToAttribute"> | ||
<_Parameter1> | ||
Blazored.LocalStorage.Tests, PublicKey=0024000004800000940000000602000000240000525341310004000001000100e94102d6760ebcff1970798791888ddf102ac709e19db9a312721fafca42b894652b59bada7d592a4ab62a5b76507a27720e922bc310c4f5aa75acd8ab59632c920ac41a7e9abcaf4b8bb5525a60931faccea704dbdcf68e1207616751447dcfec687f18854148aa66a9a09e1edc8fd0c9bd11950b4baf7d46fe38993af4add4 | ||
</_Parameter1> | ||
</AssemblyAttribute> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comment about removal of attribute
.github/workflows/ci-main.yml
Outdated
@@ -11,6 +11,7 @@ env: | |||
DOTNET_SKIP_FIRST_TIME_EXPERIENCE: true | |||
DOTNET_CLI_TELEMETRY_OPTOUT: true | |||
PROJECT_NAME: LocalStorage | |||
TESTEXTENSIONS_PROJECT_NAME: $PROJECT_NAME.TestExtensions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thinking about it, it might be better to have two separate build pipelines, one for each package. That way they can be released independently, if required.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good idea. I can work on that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I reverted the change to this file and introduced a new ci-testextensions.yml
pipeline definition (not loving the file name).
Reverts the main pipeline to just build the main Blazored.LocalStorage library. Introduces new pipeline yml for building and packaging the Blazored.LocalStorage.TestExtensions library.
Thanks for doing this @nshenoy. Apologies for taking a while to get it merged. |
Reduces app download size for projects that are dependant on Blazor.LocalStorage by not including
bunit.core
as part of the initial download. The test extensions have been pulled out into a completely separate assembly for use by unit tests. Fixes #149 .