Skip to content
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

Initial draft of sdk unified config file doc #328

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

Conversation

baronfel
Copy link
Member

@baronfel baronfel commented Jan 29, 2025

We have about 15 (/s) different configuration files and formats that rev independently by different teams. Users would be well-served by a single source of truth that all tools can read from, that has unified documentation and comes accompanied by helper libraries to ensure consistency of behavior. The first step along that journey is having a shared understanding of the config system and its intended uses, so that we can build tooling on top of that shared understanding.


## What exists today

Today there are several places where teams have put configuration for repo- or project-level configuration for tooling:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May or may not be relevant, but I'd also consider Directory.Build.rsp as another of these types of configuration sources.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly, we'll need guidance for when to put something here versus Directory.Build.props. For instance, where should I put TreatWarningsAsErrors? I think that has a surprisingly big impact for MSBuild, given that one's a default property and one's a global property.


### Nested keys

We should encourage tools to stake a claim on a top-level 'namespace' for keys in this configuration file. Keys should either be namespaced with the tool's name or be namespaced with a common prefix that is unique to the tool. This will prevent key collisions between tools and make it easier for users to understand what each key does. This allows for the format to be open to extensibility, as new tools can add their own keys without fear of collision.
Copy link
Member

@MattKotsenas MattKotsenas Jan 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider from the outset some type of namespace or "vendor" prefix, even if it's just guidance without code enforcement. I get the point of not wanting to appear to privilege our own tools over 3rd parties, however I'm also sensitive to the risk of picking suboptimal names to avoid backwards incompatibility or collisions at the top-level.

We must ensure that both the CLI and VS support the configuration file and the configuration system.

#### Usage within the dotnet CLI and supporting tools
Inside the `dotnet` CLI (and tools that ship within the CLI), we will initialize a `ConfigurationBuilder` with the `poohbear.config` file and merge it with the user's `poohbear.config` file. We will also seed that builder with environment variables. Tools that need the configuration within the `dotnet` CLI will read the appropriate values and apply them to their execution.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the tool / format starts to support env vars or other sources, consider adding a dotnet config dump or dotnet config get --all command or similar so that folks can debug setup issues.

The [git config][git-config] command is a good source of inspiration.


#### Programmatic access
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A different type of programmatic access, but should this file be available to Roslyn as well? For instance, should it be a goal, non-goal, etc. to allow something like stylecop.json or BannedSymbols.txt to be here instead?


### Composition and heirarchy

This file will be hierarchical in a very limited fashion. The final realized configuration will be the result of merging the `poohbear.config` file in the repository root with an optional `poohbear.config` file in the user's `$HOME` directory (specifically, the SpecialFolders.UserProfile location in the .NET BCL). This will allow users to set defaults for their own use, and then override those defaults on a per-repository basis for sharing with coworkers.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Somewhat silly question for me, but how do you know what the repository root is? Are you searching up for the first you find? What if they nest?

Possible options:

  • Allow nesting similar to .editorconfig, optionally with an is_root style property
  • Find the first; if so, what if the first found isn't in $HOME but is "above" the .git folder?
  • Something else?

git config doesn't have these problems because you never actually interact with the file; here it's visible to the user (which is good) but that means you can and will end up with every combination possible :)

@baronfel
Copy link
Member Author

baronfel commented Jan 31, 2025

Feedback from @KathleenDollard:

  • have a story in here about how to version defaults for this config
  • have a place for 'future work' and explicitly aim at global.json here too (the function, maybe not the form of version + roll forward configuration)

* unified opt-out of telemetry
* storing tooling decisions like which mode `dotnet test` should execute in

All kinds of repository-wide configuration need a unified place to be expressed that we lack today.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be better to extend global.json instead of introducing yet another file?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants