Skip to content

Removed nesting levels in several places through block-scoped usings#2016

Closed
Lehonti wants to merge 1 commit into
oxyplot:developfrom
Lehonti:develop
Closed

Removed nesting levels in several places through block-scoped usings#2016
Lehonti wants to merge 1 commit into
oxyplot:developfrom
Lehonti:develop

Conversation

@Lehonti

@Lehonti Lehonti commented Jul 19, 2023

Copy link
Copy Markdown

This is just reformatting for increased readability, so it should be quite safe

@Jonarw

Jonarw commented Jul 20, 2023

Copy link
Copy Markdown
Member

In general I think this is a good idea, and I am certainly not opposed to it. However I was planning to work on consolidating our code style (as a continuation of #1920) based on an updated .editorconfig in the near future, and I think it might be nice to have all style changes in one PR then.

@Lehonti

Lehonti commented Jul 20, 2023

Copy link
Copy Markdown
Author

I understand. This is the corresponding rule:

https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/style-rules/ide0063

Not sure if it's a good idea to have the IDE enforce that rule, as in some cases the old syntax may be preferred.

@VisualMelon

VisualMelon commented Jul 20, 2023

Copy link
Copy Markdown
Contributor

I'm also not opposed to these specific changes, and I agree that in some cases the old style may be preferred (obviously you've identified situations where the scope is unchanged, which presumably the editor rule will not). I'd be happy to leave this unspecified in the editor config and for us to decide on a case-by-case basis, but I don't have any strong opinions about this.

@Jonarw

Jonarw commented Jul 21, 2023

Copy link
Copy Markdown
Member

Yes, actually the automatic refactoring in VS based in the .editorconfig rule will only highlight and change using-blocks where this will not change the scope.

@VisualMelon

Copy link
Copy Markdown
Contributor

In that case, I agree we should hold this and do everything all at once with an agreeable editor config.

@Lehonti Lehonti closed this by deleting the head repository Aug 2, 2023
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.

3 participants