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

Build process maintenance 4th #1037

Merged
merged 7 commits into from
Mar 19, 2023
Merged

Conversation

9rnsr
Copy link
Contributor

@9rnsr 9rnsr commented Mar 5, 2023

Related: #1033

  • Migrate csproj for net472 to SDK style.
    Add TargetFramework net427 to *.Core.csproj files, then remove old style csprojs.

  • Fix troublesome manual .runsettings file specification for testing.
    Add default runsettings file and use it by adding <RunSettingsFilePath> setting in Directory.Build.props.
    After the SDK style csproj migrations, any tests built for all TargetFrameworks will have same relative filepaths to the required test data. Therefore we can have one test.runsettings file at the root of repository.

@9rnsr 9rnsr force-pushed the build_maintenance4 branch 4 times, most recently from 03facaa to 79769c6 Compare March 10, 2023 14:34
@9rnsr
Copy link
Contributor Author

9rnsr commented Mar 10, 2023

I fixed the failure of AppVeyor build (Internal server error).
We cannot enable generation of XML documentation files in Debug build, because it will report many many CS1591 warnings.

@tonyqus
Copy link
Member

tonyqus commented Mar 12, 2023

I don't think merge Core csproj and non-Core csproj is a good idea. I remember there are some test cases behaving differently between Core and non-Core mode.

@9rnsr
Copy link
Contributor Author

9rnsr commented Mar 12, 2023

Today, tests in Core csprojs only targets net6.0, but we can easily add one more target framework net472.

@9rnsr
Copy link
Contributor Author

9rnsr commented Mar 12, 2023

If there is no reasons other than the test behavior, I prefer to use SDK style csprojs for maintainability.

@tonyqus
Copy link
Member

tonyqus commented Mar 12, 2023

I'll evaluate if it's possible to merge Core csproj and non-Core csproj. The situation is different from before. NPOI doesn't need to support .NET framework 2.x/3.x anymore. The difference may have became very tiny.

@9rnsr
Copy link
Contributor Author

9rnsr commented Mar 12, 2023

OK, I added net472 for three test projects. After that, test explorer will display two target frameworks for each test projects. We can do testing in specific target framework, or run them all at once.

image

@tonyqus
Copy link
Member

tonyqus commented Mar 19, 2023

LGTM

@tonyqus tonyqus merged commit 88dfbbb into nissl-lab:master Mar 19, 2023
@9rnsr
Copy link
Contributor Author

9rnsr commented Mar 20, 2023

Thanks!

@9rnsr 9rnsr deleted the build_maintenance4 branch March 20, 2023 00:04
@9rnsr
Copy link
Contributor Author

9rnsr commented May 20, 2023

This pull request introduced a regression. Look at the fix in #1083.

@tonyqus tonyqus added cicd and removed enhancement labels Jun 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants