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 infrastructure updates #578

Merged
merged 20 commits into from
Oct 12, 2019
Merged

Build infrastructure updates #578

merged 20 commits into from
Oct 12, 2019

Conversation

martincostello
Copy link
Member

@martincostello martincostello commented Oct 12, 2019

  • Build the whole solution, not just specific projects and TFMs on Linux/macOS.
  • Remove redundant package reference for AWSSDK.Core as its transient from the other two.
  • Use more recent minimum version for AWS SDK.
  • Update tests to latest AWS SDKs.
  • Use a build image for Visual Studio 2019 in AppVeyor.
  • Use newer format for NuGet Symbol packages.
  • Ignore code coverage artifacts.
  • Update various NuGet packages to their latest versions.
  • Fix or suppress new analyzer warnings.
  • Update to .NET Core 3.0 SDK.
  • Update tests and samples to .NET Core 3.0.
  • Delete or disable problematic tests (see Restore test coverage to previous levels #579).
  • Switch from OpenCover to coverlet.

Build the whole solution, not just specific projects and TFMs on Linux/macOS.
Remove redundant package reference for AWSSDK.Core as its transient from the other two.
Use more recent minimum version for AWS SDK.
Update tests to latest AWS SDKs.
Use a build image for Visual Studio 2019.
Use newer format for Symbol packages.
Use the .NET Core 3.0 SDK and target netcoreapp3.0 in the tests.
Use package icon file instead of package icon URL.
Update various NuGet packages to their latest versions.
Fix analyzer warning.
Ignore code coverage artifacts.
For now, suppress code analysis warning. Can fix at a later date.
Update sample to use .NET Core 3.0.
@martincostello martincostello added this to the v7.0.0 milestone Oct 12, 2019
@martincostello martincostello requested a review from a team as a code owner October 12, 2019 21:45
@ghost ghost requested a review from jaimalchohan October 12, 2019 21:45
maurofranchi
maurofranchi previously approved these changes Oct 12, 2019
Copy link
Contributor

@maurofranchi maurofranchi left a comment

Choose a reason for hiding this comment

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

Seems ok

It's not a Regular Expression, so just use a wildcard.
Install 2.2 in Travis CI until MinVer supports netcoreapp3.0.
Dispose of IDisposable resources.
Try and fix flaky build in Travis.
Specify the same output directory.
Explicitly build for Release.
Exit if tests fail.
Disable progress indicator to try and speed up Travis CI.
Remove tests written in the "old" style that dont use the new fluent API.
Update the version of Microsoft.Extensions.Logging in the test infrastructure.
Disable flaky tests in Travis CI.
Switch from OpenCover to coverlet for .NET Core 3.0 support and cross-platform code coverage.
@codecov
Copy link

codecov bot commented Oct 12, 2019

Codecov Report

Merging #578 into master will increase coverage by 7.16%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #578      +/-   ##
==========================================
+ Coverage   38.39%   45.55%   +7.16%     
==========================================
  Files          81       89       +8     
  Lines        2930     2331     -599     
  Branches      515        0     -515     
==========================================
- Hits         1125     1062      -63     
+ Misses       1646     1269     -377     
+ Partials      159        0     -159
Impacted Files Coverage Δ
...wsTools/MessageHandling/SqsNotificationListener.cs 74.19% <100%> (+12.93%) ⬆️
src/JustSaying/CreateMeABus.cs 33.33% <0%> (-16.67%) ⬇️
...stSaying/AwsTools/MessageHandling/SqsQueueByUrl.cs 33.33% <0%> (-3.04%) ⬇️
src/JustSaying/AwsTools/ErrorQueue.cs 12% <0%> (-1.34%) ⬇️
...aying/AwsTools/QueueCreation/AmazonQueueCreator.cs 13.51% <0%> (-0.78%) ⬇️
...rc/JustSaying/Fluent/QueueSubscriptionBuilder`1.cs 0% <0%> (ø) ⬆️
src/JustSaying/Extensions/TimespanExtensions.cs 0% <0%> (ø) ⬆️
...c/JustSaying/Fluent/SqsReadConfigurationBuilder.cs 0% <0%> (ø) ⬆️
...ools/QueueCreation/ConfigurationErrorsException.cs 25% <0%> (ø) ⬆️
...c/JustSaying/AwsTools/MessageHandling/SnsPolicy.cs 0% <0%> (ø) ⬆️
... and 74 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4744d40...3b3a17a. Read the comment docs.

@martincostello martincostello merged commit abf7acb into justeattakeaway:master Oct 12, 2019
@martincostello martincostello deleted the Build-Infrastructure branch October 12, 2019 23:18
@@ -43,10 +47,21 @@
<LangVersion>latest</LangVersion>
<RepositoryUrl>$(PackageProjectUrl).git</RepositoryUrl>
<SignAssembly>true</SignAssembly>
<SymbolPackageFormat>snupkg</SymbolPackageFormat>
Copy link
Member

@slang25 slang25 Oct 13, 2019

Choose a reason for hiding this comment

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

I think I've raised this before, but this is redundant for this package as symbols are embedded.

In this file there should only be SymbolPackageFormat or DebugType, one should go. I feel that embedded is appropriate, but would understand if you disagree.

Copy link
Member Author

Choose a reason for hiding this comment

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

I changed this to guard against an issue I had with publishing one of my own packages which stil used the old format where they get published from AppVeyor to a deprecated server which has since had expired TLS cert that made publishing fail: https://ci.appveyor.com/project/martincostello/sqllocaldb/builds/28012360#L181

Even if it's not in use, setting this means if we change in the future it's future-proofed.

@slang25
Copy link
Member

slang25 commented Oct 13, 2019

Very nice 😃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants