Fix most build warnings from the projects #4125
Conversation
|
Thanks Nirmal4G for opening a Pull Request! The reviewers will test the PR and highlight if there is any conflict or changes required. If the PR is approved we will proceed to merge the pull request 🙌 |
ghost
requested review from
ghost
added
the
Nirmal4G
force-pushed
the
hotfix/warnings
branch
from
bf9fd55
to
a11e4bc
Compare
|
This PR has been marked as "needs attention 👋" and awaiting a response from the team. |
Nirmal4G
force-pushed
the
hotfix/warnings
branch
2 times, most recently
from
5606fea
to
2a0b942
Compare
|
@michael-hawker In response to #4173 (comment) I'll update the EditorConfig to include rules for Can we take this chance to formally set a style for the whole org? Can we also create a template repo with basic tools config, editor, build and CI settings? |
|
@Nirmal4G a template repo is something I've been wondering if we need too. Probably good to create an issue to discuss on the Wiki repo for tracking across the org so we can have a conversation with all stake-holders. In the meantime, is there any other things to do with this PR or can we mark it as ready for review? It's fairly small at the moment, so would be easy if it's not any bigger to merge soon for our 7.1 release as part of finalization for bugs. Otherwise we can move to the next milestone for any larger efforts. Thanks! |
Nirmal4G
force-pushed
the
hotfix/warnings
branch
from
2a0b942
to
04882b2
Compare
StyleCop warnings in generated sources@Sergio0694 Is there anyway to suppress StyleCop warnings in generated sources? See warnings I'll try to fix them. Most of them are name mismatch between filename and type ( .NET Native IL link & trim warnings@michael-hawker Do we use The warning |
@Nirmal4G Not sure, I mean, every generated source has a Anyway I wouldn't necessarily spend too much time on this given that all the .NET packages are going to a separate repo soon enough, and we'll likely end up both changing a bunch of things when we do, including style rules and configs anyway 🙂 |
Not sure, I think we used to use the ItemContainerGenerator with the old TabView, but don't see any current references to that helper class anywhere for any of our |
Nirmal4G
force-pushed
the
hotfix/warnings
branch
2 times, most recently
from
19df7a3
to
7b08511
Compare
|
@Nirmal4G was there anything else to do with this one, can we move it out of draft? Just want to know if we should merge soon as it's pretty straight-forward at the moment or move to our next milestone as we're trying to close out the release? |
Code CS1591: Missing XML comment for publicly visible type or member.
Code ILT0010: Could not find an assembly referenced by other assembly The following warnings surfaced due to the missing assembly. Since, the assembly is not found, any referenced code got hit with these warnings Code ILT0005: The type from an assembly was not included in compilation, but was referenced in a method. Code ILT0003: The method will always throw an exception due to the missing method in an assembly.
Code SA1208: Using directive should appear before other directives. Code SA1210: Using directives should be ordered alphabetically by the namespaces. Code SA1505: An opening brace should not be followed by a blank line.
Add comments describing the nature of the warning.
Code CS1591: Non-nullable field must contain a non-null value when exiting constructor.
Nirmal4G
force-pushed
the
hotfix/warnings
branch
4 times, most recently
from
e8df919
to
19c62a3
Compare
|
@michael-hawker All done here. I'm still investigating |
There was a problem hiding this comment.
Not a fan of the blank lines between using directives as we're not doing that anywhere else, but also don't want to nitpick and delay a PR that is mostly just tweaking the style of some unit tests, so it's fine. Looks good, thanks for removing those warnings! 🙂
|
I didn't add new lines at first, when I did add them and compare the two, the one with the new lines looked best. So, I kept them. If we want, the next time we touch this file, we could clean it up. But IMHO it sure does look better now! |
Partially Fixes #4240
Fix most warnings from the projects! Only warnings left are MVVM from source generators.
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Build produces so many warnings. It is a source of developer's frustration and shame! 😢
What is the new behavior?
Build now produces less impactful or no warnings at all. Happy CI, Happy Dev! 😎
PR Checklist
Please check if your PR fulfils the following requirements:
Other information
rebaseon latestHEADand then commit, without updating from the latestHEAD.Merge pull request #xxxx from repo/branch, and commit message to either PR message or messages of individual commits. Theauto-mergebot does this by default.