-
Notifications
You must be signed in to change notification settings - Fork 751
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
Treat Build Warnings as Errors for Abstractions & DependencyInjection Projects #4050
Conversation
… up the IUserInfo to remove build warnings
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.
💪🏻
@@ -1,33 +1,38 @@ | |||
// Licensed to the .NET Foundation under one or more agreements. | |||
// The .NET Foundation licenses this file to you under the MIT license. | |||
// See the LICENSE file in the project root for more information | |||
|
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.
Is this blank line required? I had been removing this line. I'm fine either way, just wondering if I was working against StyleCop when I was doing 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.
It is not a stylecop setting, for some reason I thought it was inside our stylecop configuration. I can remove these blank lines below the file header.
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.
To be clear, what would the reviewers like me to do here? White space or no white space
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 would prefer no white space, but happy to be overridden if there are any other opinions
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 have removed the whitespace, I'm going to leave this conversation open if anyone has strong opinions on this
Co-authored-by: Brian Dukes <bdukes@engagesoftware.com>
OMG @valadas - who does 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.
Looks good to me
Fixes #4040
Summary
This PR updates
DotNetNuke.Abstractions
andDotNetNuke.DependencyInjection
to treat all build warnings as errors. This will help ensure a high level of code quality in these new projects.