.NET Core 3.0 #521
.NET Core 3.0 #521
Conversation
|
this PR requires changes to appveyor scripts because the .net framework and .net core unit tests can't be run with the same commands. working on it... |
|
i got the unit tests to run in appveyor, so this is ready for review. there seems to be some issue getting the code coverage report to codecov but I'm not sure if it's a fluke or a problem with the appveyor script |
|
Messaged the CodeCov team about the issue in showing the report. I didn't see anybody getting the same issue on a quick Google search. |
Test/TestVariantProtein.cs
Outdated
| ).ToList(); | ||
| ProteinDbWriter.WriteFastaDatabase(variantProteins2.OfType<Protein>().ToList(), @"E:\ProjectsActive\Spritz\mmTesting\variantproteins.fasta", "|"); | ||
| } | ||
| // this test references the class ProteinWithAppliedVariants which doesn't seem to exist? |
There was a problem hiding this comment.
Weird. Yes, this test class is old and can be deleted.
ProteinWithAppliedVariants is just Protein with some additional info now.
| <file src="ThermoRawFileReader\bin\Release\net471\ThermoRawFileReader.dll" target="lib\net471" /> | ||
|
|
||
| <file src="mzLib.targets" target="build\net471" /> | ||
| <file src="BayesianEstimation\bin\Release\netcoreapp3.0\BayesianEstimation.dll" target="lib\netcoreapp3.0" /> |
There was a problem hiding this comment.
Did you test packaging the nupkg and then using it in MetaMorpheus to check that it works?
There was a problem hiding this comment.
yep! I had to make a .NET core 3 metamorpheus build, but all is OK. I'd like to test more on a Linux machine but that's not necessarily required to merge the PR. I just want to test getting computer diagnostic info on different operating systems.
| <Project ToolsVersion="4.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003"> | ||
| <Target Name="AfterBuild"> | ||
| <ItemGroup> | ||
| <MyPackageFiles Include="$(MSBuildProjectDirectory)\..\Packages\**\build\net471\UnmanagedThermoHelperLayer.dll"/> |
There was a problem hiding this comment.
It's nice to see we can get rid of this. It made the build more confusing.
|
I think the codecov thing has something to do with the .pdb files. By default, release builds don't create the full .pdb file, and the full .pdb file is necessary for codecov to function properly. This tag should cause the full .pdb to build: The subsequent codecov command in the build script works on my local machine (meaning, codecov determines the code coverage from the OpenCover .xml) but it doesn't work on appveyor; not sure why. |
| /// <summary> | ||
| /// see https://github.com/NimaAra/Easy.Common/blob/master/Easy.Common/DiagnosticReport/DiagnosticReport.cs | ||
| /// </summary> | ||
| private static string AsBashCommand(string command) |
There was a problem hiding this comment.
pretty simple but would a unit test for this be good?
There was a problem hiding this comment.
This function only gets run on OSX, so on other machines the unit test would fail (including on AppVeyor). It might work on Linux. I don't think it's easily unit-testable
There was a problem hiding this comment.
You could test that it fails if the environment is windows
Proteomics/Fragmentation/Product.cs
Outdated
| @@ -50,7 +51,7 @@ public string Annotation | |||
| /// </summary> | |||
| public override string ToString() | |||
| { | |||
There was a problem hiding this comment.
could pass int into ToString(n=11) to adjust rounding. not vital
Test/TestVariantProtein.cs
Outdated
| ).ToList(); | ||
| ProteinDbWriter.WriteFastaDatabase(variantProteins2.OfType<Protein>().ToList(), @"E:\ProjectsActive\Spritz\mmTesting\variantproteins.fasta", "|"); | ||
| } | ||
| // this test references the class ProteinWithAppliedVariants which doesn't seem to exist? |
rmillikin
dismissed stale reviews from trishorts, leahvschaffer, and acesnik
via
50631fe
No description provided.