Nunit-console: Remove Rebracer file in favour of .editorconfig

[ChrisMaddock]

@ChrisMaddock commented on Thu Jan 25 2018

We currently have three different files providing formatting guidelines - which I don't believe are all maintained.

The most recent work has been done in .editorconfig, I believe. Do we have any need to maintain all three files, or could we remove the Rebracer and ReSharper options?

I'd like to apply the same change to both the nunit and nunit-console repo's, if we can make a decision.

---

@rprouse commented on Thu Jan 25 2018

Agreed. Rebracer isn't needed in VS 2017 which is now required to build so can be dropped. We also haven't updated Resharper in years other than your automatic changes.

---

@jnm2 commented on Thu Jan 25 2018

I was just looking at that. I think we should remove them but be open to adding minimal ReSharper settings back in to quiet down ReSharper's default complaints in our codebase for things that .editorconfig doesn't control yet.

---

@ChrisMaddock commented on Thu Jan 25 2018

That sounds like a good idea to me. 😄

---

@zastrowm commented on Fri Jan 26 2018

Note that ReSharper also has support for configuring it via .editorconfig: https://www.jetbrains.com/help/resharper/EditorConfig_Index.html

---

@ChrisMaddock commented on Fri Jan 26 2018

Thanks @zastrowm!

Looks like we've made a decision. Should anyone new to contributing fancy picking this up, to clarify, the task is to remove the Rebracer.xml and NUnit.sln.DotSettings files from the repository. Pull Requests Welcome!

[ChrisMaddock]

In this repo, it would also be good to update the .editorconfig file to match https://github.com/nunit/nunit/blob/master/.editorconfig

[mikkelbu]

Should we keep the .DotSettings similar to what we noted in nunit/nunit#2685 (see
https://github.com/nunit/nunit/issues/2685#issuecomment-360794664)

[rprouse]

I think we should keep the .dotsettings. This project still compiles in 2015, so the full editorconfig won't replace the .dotsettings.

[ChrisMaddock]

Is editorconfig only from VS 2017? I didn’t know that.

Agreed - let’s just remove Rebracer.xml

[rprouse]

Editorconfig is built into VS2017. Prior to that, you need the editorconfig extension installed and it does not support the C# extensions to editorconfig, just spacing, line endings, etc.

[lennartb-]

Is this still a current issue? It seems to have been removed with https://github.com/nunit/nunit/issues/2685 from the main project, and here only the Rebracer.xml seems to be still here. I grepped the project and there don't seem to be any references anymore. I'd have a PR ready.

[mikkelbu]

@lennartb You are most welcome to tackle this task. But I think that there is more to this task than just removing Rebracer.xml. We should also update the .editorconfig to match the one in the NUnit project - https://github.com/nunit/nunit/blob/master/.editorconfig.

Also since quite some time has passed since this issue was last discussed, I think we should either remove the DotSettings file or at least make it match the one in the NUnit project - https://github.com/nunit/nunit/blob/master/NUnit.sln.DotSettings.

[lennartb-]

Done, there are few violations of the rules (but they don't emit build warning), should these be fixed according to https://github.com/nunit/docs/wiki/Coding-Standards#naming as well?

[mikkelbu]

@lennartb- Can you give some examples on these violations? But personally (without knowing the issues), I think we can solve these in later PRs (also to keep each PR limited in scope)

[lennartb-]

It's mostly variables beginning with an underscore (when they shouldn't) and protected fields that should begin with an upper-case letter (or rather, be converted into properties).

[mikkelbu]

If that is the case, then I think it is fine to leave them as they are, and then we can improve on this later on.

[jnm2]

The Roslyn team is working on making .editorconfig warnings and errors show up in command line builds, so one day this will fail CI and force the issue.