microsoft / XamlBehaviors

This is the official home for UWP XAML Behaviors on GitHub.
MIT License
700 stars 112 forks source link

Added .editorconfig, avoid conflicts with personal C# Code Style settings #139

Closed sonnemaf closed 5 years ago

sonnemaf commented 6 years ago

You get a lot of warnings if you have a conflicting personal VS2017 C# Code Style. For example if you have setup braces on the 'same line' and not on the 'next line'. This .editorconfig file overrides the personal settings which avoids these warnings.

msftclas commented 6 years ago

CLA assistant check
All CLA requirements met.

mgoertz-msft commented 6 years ago

Did you generate this file? A lot of the settings are either hard to recognize be name or perhaps even wrong. For example, I don't think we use var in here. Perhaps we should reduce this down to just the most meaningful ones?

pedrolamas commented 6 years ago

Did you generate this file?

I have a feeling this was generated directly from VS2017! 😃

https://twitter.com/fonssonnemans/status/1056276686264770560

sonnemaf commented 6 years ago

I generated it using the Visual Studio 2019 Preview. Next I added the naming sections (lines 139 and below) to it.

It is hard to decide what the meaningful settings are. You will than still have conflicts with your personal settings in Visual Studio (menu Tools, Options, Text Editor, C#, Code Styles). That's why I chose all. Some are only supported in VS2019 but are for now ignored in VS2017.

I have chosen a somewhat neutral settings with only a few warnings (mostly suggestions, sometimes silent). We must finetune them for this project. But it is a beginning.

mgoertz-msft commented 6 years ago

Let's make sure that these settings at least encourage our coding guidelines for this repo:

While I don't know how to even specify the middle one we should probably makes the others suggestions, i.e. my PR suggestions for var should actually also be of type suggestion.

LocalJoost commented 6 years ago

Interesting. I was not even aware of these coding guidelines. I am also very against #3. My opinion is state once and only once if possible. I mean List bla = new List() …. Meh

In case of Var bla = SomeObject.SomeMethod(xyz) … yeah, I might use an explicit type in stead of a var.

Joost van Schaik

Windows Development MVP Sent from mail for Windows 10

"The world as I envision it in my head is such a more interesting place"


From: Marco Goertz notifications@github.com Sent: Monday, October 29, 2018 11:20:40 PM To: Microsoft/XamlBehaviors Cc: Subscribed Subject: Re: [Microsoft/XamlBehaviors] Added .editorconfig, avoid conflicts with personal C# Code Style settings (#139)

Let's make sure that these settings at least encourage our coding guidelineshttps://github.com/Microsoft/XamlBehaviors/wiki/Contributing-Guidelines for this repo:

While I don't know how to even specify the middle one we should probably makes the others suggestions, i.e. my PR suggestions for var should actually also be of type suggestion.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHubhttps://github.com/Microsoft/XamlBehaviors/pull/139#issuecomment-434102493, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AD8Bn0xAEKmYdKbVxFrSOdxVy3AkSZOyks5up384gaJpZM4X_c0J.

mgoertz-msft commented 6 years ago

I fall into the being rather explicit camp. How about we set the var style to silent as suggested above?

LocalJoost commented 6 years ago

Amen

Joost van Schaik

Windows Development MVP Sent from mail for Windows 10

"The world as I envision it in my head is such a more interesting place"


From: Marco Goertz notifications@github.com Sent: Tuesday, October 30, 2018 5:57:28 PM To: Microsoft/XamlBehaviors Cc: Joost van Schaik; Comment Subject: Re: [Microsoft/XamlBehaviors] Added .editorconfig, avoid conflicts with personal C# Code Style settings (#139)

I fall into the being rather explicit camp. How about we set the var style to silent as suggested above?

— You are receiving this because you commented. Reply to this email directly, view it on GitHubhttps://github.com/Microsoft/XamlBehaviors/pull/139#issuecomment-434382429, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AD8Bn7KoxIyz9X7Z_drejzHprDRreb7rks5uqIT4gaJpZM4X_c0J.

sonnemaf commented 6 years ago

The coding convention says 'Avoid var' so a silent suggestion should be ok. Can the PR be merged?

mgoertz-msft commented 6 years ago

Oh, I was still waiting for my suggestions above to be included in the PR, i.e. make this. suggestions and change var to false but keep it silent.

mgoertz-msft commented 6 years ago

@sonnemaf I know it pains you but are you still planning on flipping the var settings to false:silent? :-)

skendrot commented 6 years ago

I would love to change the this. prefix and bring in the underscore prefix for private member variables. I find it very confusing when variables are not underscore prefixed.

private int myValue;

private void Foo(int myValue)
{
    // wait, now we have two myValue variables!!?!?
}
LocalJoost commented 5 years ago

I completely agree with Shawn.

Joost van Schaik

Windows Development MVP Sent from mail for Windows 10

"The world as I envision it in my head is such a more interesting place"


From: Shawn Kendrot notifications@github.com Sent: Friday, November 2, 2018 12:08:25 AM To: Microsoft/XamlBehaviors Cc: Joost van Schaik; Comment Subject: Re: [Microsoft/XamlBehaviors] Added .editorconfig, avoid conflicts with personal C# Code Style settings (#139)

I would love to change the this. prefix and bring in the underscore prefix for private member variables. I find it very confusing when variables are not underscore prefixed.

private int myValue;

private void Foo(int myValue) { // wait, now we have two myValue variables!!?!? }

— You are receiving this because you commented. Reply to this email directly, view it on GitHubhttps://github.com/Microsoft/XamlBehaviors/pull/139#issuecomment-435216769, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AD8Bnxl1fgh5coj0nLsnKrn9UWJYKXXtks5uq37pgaJpZM4X_c0J.

pedrolamas commented 5 years ago

I'm also with @skendrot on this, I would prefer to just prefix private fields with _.

However, that means we need to change the coding guidelines also, and probably do a full refactoring/replace of the code to ensure it conforms with this!

brianlagunas commented 5 years ago

I am also with @skendrot _ FTW

sonnemaf commented 5 years ago

+1 for the _ prefix for private fields, but not a warning. A suggestion or maybe silent.

skendrot commented 5 years ago

Sounds like we have a majority! I'm happy to do the work to fix this awful mistake ;)

LocalJoost commented 5 years ago

Make it so Mr Spock 😉

Joost van Schaik

Windows Development MVP Sent from mail for Windows 10

"The world as I envision it in my head is such a more interesting place"


From: Shawn Kendrot notifications@github.com Sent: Friday, November 2, 2018 3:03:05 PM To: Microsoft/XamlBehaviors Cc: Joost van Schaik; Comment Subject: Re: [Microsoft/XamlBehaviors] Added .editorconfig, avoid conflicts with personal C# Code Style settings (#139)

Sounds like we have a majority! I'm happy to do the work to fix this awful mistake ;)

— You are receiving this because you commented. Reply to this email directly, view it on GitHubhttps://github.com/Microsoft/XamlBehaviors/pull/139#issuecomment-435389878, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AD8Bn-rGtzif_OjtQbT2U08cJkjJMHS-ks5urFCZgaJpZM4X_c0J.

mgoertz-msft commented 5 years ago

OK, it's a community project and the community has spoken. Let's change it and feel free to turn these settings into suggestions to help adhere to the new guidelines.