-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Define spec for NuGet packages providing .editorconfig defaults #19028
Comments
(FYI to @sharwell) |
The semantics I'm considering would be the following:
Open questions:
|
Your last two questions are the ones that interest me most. |
@jasonmalinowski Are NuGet packages referenced by a project ordered when using |
MSBuild ItemGroups implicitly have an order. Whether that order is preserved through the tooling is definitely not something I'd bet a dollar on. |
In relation to what @sharwell proposed above:
I suggested something different in the proposed solution in #29592:
As for the question:
See this linked stylecop example also mentioned in #29592 |
Any update on this? Being able to centralize |
It seemed like we had some luck with including one in a NuGet package, but now I'm now sure if it's quite working consistently. That said, I have some thoughts on how this might work and be prioritized. Here's what I propose: A new MSBuild ItemDefinition, EditorConfig, with Priority (for sorting the layering as it would if they were in different tiers of the hierarchical folder tree) and ConfiguredPath Metadata (for specifying directories deeper than the project file, or maybe for specifying configuration for source files included in the NuGet package). A NuGet package could package the file in their build/buildTransitive directory along with a props/targets file that Includes the EditConfig Item. I think by default an .editorconfig in the project directory or a configured path would apply last and override any settings supplied by a NuGet package; maybe this would be done by having the Microsoft.NET.Sdk glob **/.editorconfig with a very high Priority number? Perhaps something predictable so if someone wanted to make a package that overrides local settings it could? Perhaps other items would default to 0 and simply apply in the order they are processed. Here's a thought of what a Sample.Package.targets file would look like: <Project>
<PropertyGroup>
<SamplePackageEditorConfigPriority Condition="'$(SamplePackageEditorConfigPriority)' == ''">0</SamplePackageEditorConfigPriority>
</PropertyGroup>
<ItemGroup>
<EditorConfig Include="@(MSBuildThisFileDirectory)Sample.Package.editorconfig">
<ConfiguredPath>$(MSBuildProjectDirectory)</ConfiguredPath>
<Priority>$(SamplePackageEditorConfigPriority)</Priority>
</EditorConfig>
</ItemGroup>
</Project> Thoughts? |
I'm confused. I thought allowing a custom location was already allowed in VS 16.7? |
I wish they would document this stuff, least I missed it. So, I have had a colleague try to do this. He says it definitely works for C# rules but not others. For instance we had a rule that states csproj files must use indent size 2. This apparently has no effect:
So it is a partial solution, meaning we can centralise the important stuff (so thank you for sharing) but I'd like to see it support all rules. Also to note, we had issues with |
I raised an issue against the docs for it a while back, hopefully it'll get added soon. https://github.com/MicrosoftDocs/visualstudio-docs/issues/6051 |
While this is still under discussion, are there any known workarounds for sharing an Should I'm not a fan of creating a dependency between repositories like that, but I fail to see any other alternative at this point. Would be interested if anybody has come up with a different solution. |
I am also curious about some workaround, as I am currently in the process of migrating a giant Perforce Monorepo to Git. This scenario hit me right now, because all projects were referencing a single ruleset file (which is deprecated anyway now) in this Monorepo. Splitting those up to multiple repos create the need for distributing those rules (= .editorconfig files) to multiple repos. From what I've read Distributing the file via NuGet package sounds interesting, but it would create the need for creating a custom build target that copies the file back to the project root on build, or am I wrong here? A minimalistic solution would be best, because those changes quickly affect a very large number of repos |
@deuko i feel like this should be doable somehow, perhaps leveraging a symbolic link that references the editor config inside the submodule folder in the root of the repository. IIRC you can have symbolic links merged into the repo and apply automatically to all developers. It's far from ideal but maybe it's an option. I have not tried it though as I've since changed projects into a monorepo solution. |
@deuko I copy on build as a workaround for now. |
We achieved it with .props and .targets file with our CodingGuideline nuget. We added everything into two And added a check that the So, everything are in the |
@cmenzi Do you have an example of this? |
This would generally be a request for dotnet/msbuild, but in this case solution files are already treated as a special kind of project file. If you build with |
@jasonmalinowski @mavasani @chsienki hasn't this item been completed via globalconfig files? |
@jmarolf Is there a document describing the semantics for .globalconfig when it comes from a NuGet file? Specifically, I'd be looking for a document explaining how rules in a .globalconfig coming from a NuGet package can be overridden by a .globalconfig in the solution directory without reporting warnings about multiple configurations for the same key. Once the spec exists, we can link it here and close the issue. |
@sharwell I believe you can get some information from #49834. Tagging @chsienki and @gewarren as well - we should add information about |
I've created a docs issue here: dotnet/docs#24312. I'm happy to take a stab at it and you can review, or the other way around @chsienki. |
I created a very simple example of building distributable .editorconfig defaults. I also published it to NuGet. |
Eslint solves this by having a formal "extends" semantic in the config. {
"extends": [
"eslint:recommended",
"plugin:@typescript-eslint/recommended"
],
} Ideally the editor config would have a similar semantic, which would have knowledge of how to resolve configs from nuget packages, perhaps something like: root = true
extends = nuget:ExampleSharedPackage That would resolve that package reference, and then look for a |
If there is a way to share Also, you’re already doing something to keep your various projects’ |
after several hours of trail and error I ended up with the following solution/workaround (I hope this helps someone):
{PackageId}.props
{PackageId}.nuspec
{PackageId}.globalconfig
note: you can overwrite rules provided by the package using an .editorconfig file in the target solution |
Awesome, had no idea that was there. Thank you for dropping that here. |
I'm extremely happy to see the new Hopefully the transition to this new format will finally allow us to have first-class support for "solution packages". |
Can you tell us more? Where did you hear about it if there is no docs? And most importantly when? :) |
About Visual Studio 2022 17.10 Preview 3 added this feature. Some videos: |
I got to know about it from Nick's video (posted by @anreton above).
Oh, was it already available on 3? I guess that makes sense... Apparently, Preview 4 dropped yesterday (release notes are up now) |
Directory.Build.props do not work for you? Not convenient, but it works. |
I would suggest reading through this whole thread here. If the new |
The package can provide a .globalconfig instead. These files are not bound by the hierarchical nature of the file system. All of our own features that provide configuration as part of a NuGet package do so with .globalconfig files. |
Seeing there's some revitalized conversation here about We shouldn't expect .globalconfig to be usable for specifying things like indentation/new lines/brace preferences and other non-analyzer editor settings, right? As for
I do agree with this sentiment, though perhaps its a bit tangential to this conversation. Having an option like this would make for a much better experience for several scenarios we encounter, some of which are currently (less-than-ideally) "solved" using Command Task Runner for pre/post-solution build events (which do not even know if the build succeeded or not). Incrementing local dev package versioning (particularly in non-git code bases) can be challenging to orchestrate at the project-level because each project calculates its own version but uses that version for all dependencies on packages inferred from project references, though when those projects build the incrementing code has a difficult time sharing previously calculated information and so it calculates a new version and then the packages in a single solution build do not correctly use the version numbers of packages actually produced. Doing this outside in command task runner at least allows a script to calculate the version once and then pass it in to all projects, but the task runner explorer extension is not exactly actively maintained and does not work consistently so we consider the mentioned approach a workaround we're trying to get away from. Many of our challenges in this regard come from local development workflows in a code base where many separate solutions produce outputs which need to be composited together. Other examples include:
I'm sure there's more, but those are the ones that first come to mind off the top of my head. |
@MisinformedDNA Just going through and looking at messages I may have missed on this thread, I just took a look at your package and am wondering if this still works today? It looks like it might solve the problem we're looking to solve the way we're trying to solve it. |
There are some scenarios that exist where a NuGet package should be able to provide "default" .editorconfigs for a project. Consider the scenarios of:
We need to design the following parts:
For the latter, NuGet content support might be sufficient since we could define a new item group that it produces items for.
The text was updated successfully, but these errors were encountered: