-
-
Couldn't load subscription status.
- Fork 91
Fix warnings in src package #488
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
base: master
Are you sure you want to change the base?
Conversation
|
Thank you! Will look later. |
|
I am quite ashamed to say that I can't explain how these warnings show up in my project and not in yours. I do use Polysharp which comes with some polyfills and analyzers but even when removing u still see some. SDK is the same too. But when I find out I think I would try to add a change here to reveal them. I think it's important for the src packages because in the end of the target project can't build these it's useless. At the same time ignoring all these like I did in this PR is fine because this is the same code as the non-src ones so it's as good. Ideally the BuildInternal script would inject code to ignore all warnings, this way you do what you want and these why will work anywhere. |
|
I asked chatgpt and I think I found the solution. Update incoming. Much simpler, and hassle-free for you. |
345f73c to
93a0900
Compare
93a0900 to
eddbc4f
Compare
| { | ||
| $content = Get-Content -path $file | ||
| $content = $content -creplace "public(?=\s+(((abstract|sealed|static|record)\s+)?(partial\s+)?class|delegate|enum|interface|struct|record))", "internal" | ||
| $content = ,"#pragma warning disable" + $content # $content is a list of lines, insert at the top |
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.
This generates the #pragma warning disable for each internal files.
This way the warnings are not ignored in your standard code, in case you want to see and handle them individually
| protected internal virtual Expression VisitDebugInfo(DebugInfoExpression node) => node; | ||
| } | ||
|
|
||
| #nullable restore |
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.
This is not necessary at the end of files, these directives are per-file.
| @@ -1,4 +1,6 @@ | |||
| using System; | |||
| #nullable disable | |||
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.
Added #nullable disable for every file, since this will be necessary for internal files, and you really don't use nullable types.
|
|
||
| [AttributeUsage(AttributeTargets.Parameter, Inherited = false)] | ||
| class CallerArgumentExpression : Attribute | ||
| class CallerArgumentExpressionAttribute : Attribute |
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.
There would be a conflict otherwise in internal files in some fwks, it would be conflicted between this and the real ones when it's used later in the code.
| public static StringBuilder Argument(StringBuilder sb, int ordinal) => sb.AppendFormat(CultureInfo.InvariantCulture, "V_{0}", ordinal); | ||
| public static StringBuilder Label(StringBuilder sb, int offset) => sb.AppendFormat(CultureInfo.InvariantCulture, "IL_{0:D4}", offset); | ||
|
|
||
| public static StringBuilder MultipleLabels(StringBuilder sb, int[] offsets) |
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.
Warnings could be ignored, but I think it's good to set the culture in case there is a side effect when rendered "technical" values like these.
| public static bool AllowPrintCS = false; | ||
| public static bool AllowPrintExpression = false; | ||
| public static bool DisableAssertOpCodes = false; | ||
| public static bool AllowPrintIL; |
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.
These were valid warnings appearing in my build. Though doesn't matter much in this case.
|
I am happy with these changes now. The main idea is to inject |
No description provided.