EditorConfig and spelling_exclusion.dic added #134
Conversation
WalkthroughThe recent update to the Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@frobijn A big bonus would be also adding a Directory.Build.props at solution level and enable nullable reference types + recommended analysis mode. However, depending on the project it might prove unrealistic to apply recommendations across the board. However as a template for new projects it might serve a purpose. **edit
|
|
Indeed it is possible to add a lot more rules/analysis to a project nowadays. That has the advantage that you'll get feedback while typing code, rather than having to wait until a build (like StyleCop that seems to be used in some repositories). And that the IDE will offer a list of actions (for some rules) to fix the deviation with two mouse clicks. It may be an improvement, don't know how the core team thinks about that. As this is not a priority for me at this moment and I'm not prepared to do any work on this, I did not propose it. The .editorconfig is different as not having an .editorconfig and having different VS-settings made it impossible for me to apply small changes to the source code. BTW I think that the nanoFramework has sufficient opportunities (= MSBuild extensions) to accommodate global analysis properties. I'm also a big fan of nullable-enabled. But it only makes sense if the C# code is updated to reflect that, otherwise you'll drown in warnings. I once switched a project to nullable-enabled, it took more time than anticipated. |
|
@frobijn here's the .NET official editorconfig: I would also add works like nanoFramework (mind de case) to the dictionary |
- Kept end_of_line and charset for all files, as those are not in the runtime .editorconfig - Kept spell checker settings
|
Done, see commit. Learned a new trick - file_header_template. Nice. The spell checker has apparently been adapted for use in code. It knows about camel casing, so it interprets nanoFramework as two words nano and Framework. The word "nano" is already present in the spelling_exclusion.dic. I don't know of a way to instruct the spell checker that NanoFramework is incorrect and to suggest nanoFramework instead. I'm currently nosing around in the TestFramework repository, I'll modify and use the .editorconfig there as well. If you are ready to use the .editorconfig in more repositories, I can create a pull request for that one. |
|
So far Ive encountered one difference between the dotnet rules and nanoFramework. In dotnet static fields are prefixed by |
|
Yes, file_header_template is neat 😉 |
Let's go along the .NET code style. Please update the coding style doc. |
This reverts commit a705a09.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (5)
.editorconfigis excluded by none and included by nonenanoFramework.Networking.Sntp/Sntp.csis excluded by none and included by nonenanoFramework.Networking.Sntp/nanoFramework.Networking.Sntp.nfprojis excluded by none and included by nonenanoFramework.Networking.Sntp/packages.configis excluded by none and included by nonespelling_exclusion.dicis excluded by none and included by none
Files selected for processing (1)
- nanoFramework.Networking.Sntp/packages.lock.json (1 hunks)
Additional comments not posted (1)
nanoFramework.Networking.Sntp/packages.lock.json (1)
5-10: New dependency entry forMicrosoft.CodeAnalysis.NetAnalyzerslooks correct.The type is correctly specified as
Direct, the requested and resolved versions match (8.0.0), and the content hash is properly formatted.
|
@frobijn I was experimenting with enforcing code style and rules on build. I've pushed my latest changes here. |
|
|
@frobijn don't want to distract you, still wondering if you had the opportunity to check the above. Can you confirm that is not working? Perhaps you've found a workaround... |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
nanoFramework.Networking.Sntp/packages.lock.json (1)
5-10: Excellent addition of Microsoft.CodeAnalysis.NetAnalyzers!The inclusion of
Microsoft.CodeAnalysis.NetAnalyzers(version 8.0.0) is a valuable addition to the project. This analyzer package will help enforce coding standards, catch potential issues early, and improve overall code quality. The exact version pinning and content hash inclusion are good practices for maintaining consistency and security.This addition aligns well with the PR objectives of improving the development experience and coding standards. It's a step towards more robust and maintainable code.
Consider documenting any specific analyzer rules or configurations you plan to use in the project's README or a separate coding standards document. This will help developers understand and adhere to the project's coding guidelines.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (2)
nanoFramework.Networking.Sntp/nanoFramework.Networking.Sntp.nfprojis excluded by none and included by nonenanoFramework.Networking.Sntp/packages.configis excluded by none and included by none
📒 Files selected for processing (1)
- nanoFramework.Networking.Sntp/packages.lock.json (1 hunks)
I hadn't noticed your message. I'll look into it. |
|
Indeed that will not work with nanoFramework projects. I've looked at the microsoft.codeanalysis.netanalyzers package (in %userprofile%.nuget\packages\microsoft.codeanalysis.netanalyzers\8.0.0), The package has all its MSBuild stuff in the buildTransitive directory. That is a NuGet 5+ feature; NuGet 4- packages should use build. In terms of Visual Studio projects, NuGet 4- is used by projects that use packages.config file, and NuGet 5+ by projects that use the PackageReference in the project file. But wait! There is more! Microsoft claims you can use PackageReference in old-style projects as well. Visual Studio provides an upgrade option that does not work for *.nfproj files. But you can do that by hand and remove the packages.config and packages.lock: For the build that works: the nanoFramework.CoreLibrary is included and the build succeeds. However a lot of other things do not work. If you want to manage the NuGet dependencies in Visual Studio, no packages are visible. The packages.lock.json is no longer created. I stopped there - even if you could get this to work for MSBuild, then the auto-update message "PackageReference is not supported for this project" is probably correct. Not sure if it is because the project is an old-style project or because it is not one of the well-known Microsoft projects. |
|
We can't use PackageReference at this time, so I suppose this puts an end to the question. |
|
I've created a new PR with only the .editorconfig/spelling_exclusion.dic: #138. That was easier than continuing this branch. |
Description
Added an .editorconfig file with the settings required to support auto-formatting in Visual Studio. Als included a spelling_exclusion.dic with the words that are not part of the default dictionary of the spell checked.
Update to the documentation: see nanoframework/nanoframework.github.io#203
Motivation and Context
See discussion in #tools in Discord. The .editorconfig file is the only supported way (by Microsoft) to add repository-specific settings that control the behavior of the Visual Studio editor - as an alternative to changing the global Visual Studio settings. It is the only way to configure the spell checker.
How Has This Been Tested?
Types of changes
Checklist:
Summary by CodeRabbit
New Features
Chores