Skip to content

Update configuration parsing logic#10361

Merged
f-alizada merged 8 commits intodotnet:mainfrom
f-alizada:dev/f-alizada/update-configuration
Jul 24, 2024
Merged

Update configuration parsing logic#10361
f-alizada merged 8 commits intodotnet:mainfrom
f-alizada:dev/f-alizada/update-configuration

Conversation

@f-alizada
Copy link
Copy Markdown
Contributor

@f-alizada f-alizada commented Jul 12, 2024

Closes #10232, Fixes: #10315

Context

Current implementation of the configuration closely connected to the naming of the Enum and its values.
For instance, in order to configure the EvaluationAnalysisScope of the BuilcCheck with the behavior ProjectOnly the .editorconfig file will look like that:

[*.csproj]
build_check.rule_id.EvaluationAnalysisScope = ProjectOnly 

which makes it impossible to set the configuration name and values different from the classes names, hence complicates the configuration time from user perspective.

Changes Made

  • Decoupled the BuildCheck behaviour configuration values from the EditorConfig configuration options. Now it is possible to make many to 1 mapping between the editorconfig and the value if needed:
graph LR
scope.editor_configuration_1 --> BuildCheckBehaviourValue
scope.editor_configuration_2--> BuildCheckBehaviourValue
Loading
  • Updated documentation with replacing the CamelCase configuration to snake_case (documentation preview mode)
  • Updated the configuration values of EvaluationAnalysisScope

Testing

  • Manual sanity check 1 test
  • Current test coverage of the build check
  • Updated the configuration test to reflect the changes of the evaluation scope configuration name

@f-alizada f-alizada marked this pull request as ready for review July 13, 2024 10:43
Copy link
Copy Markdown
Member

@JanKrivanek JanKrivanek left a comment

Choose a reason for hiding this comment

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

LGTM.

I'd maybe prefer BCL parsing where possible, but it's not a blocker

Copy link
Copy Markdown
Member

@surayya-MS surayya-MS left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@f-alizada f-alizada merged commit 48e81c6 into dotnet:main Jul 24, 2024
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.

Multiword configuration options renaming [Feature Request]: Expand BuildCheck editorconfig allowable syntax

4 participants