Skip to content

Add .editorconfig file #398

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

Closed
ErikSchierboom opened this issue Sep 4, 2017 · 4 comments · Fixed by #692
Closed

Add .editorconfig file #398

ErikSchierboom opened this issue Sep 4, 2017 · 4 comments · Fixed by #692

Comments

@ErikSchierboom
Copy link
Member

This will help us with code consistency. See: https://almvm.azurewebsites.net/labs/tfs/editorconfig/

@jpreese
Copy link
Contributor

jpreese commented Sep 5, 2017

I'll need to look into editorconfig a bit. My immediate concern is that its going to clash with #203 which is currently attempting to leverage StyleCop.

@robkeim
Copy link
Contributor

robkeim commented Sep 5, 2017

I haven't heard of editorconfig before, but it looks like it's really simple to use. It isn't nearly as exhaustive as what StyleCop can provide. That said, if you look at the Rosyln project's eidtorconfig here which was linked in the link @ErikSchierboom added above it still looks pretty detailed.

The advantage that I see with editorconfig is that in VisualStudio you can clean up the code according to the editorconfig rules with a keycord combination which is something StyleCop doesn't support. I'm not sure if we could integrate into the Travis build system or not though.

@felix91gr
Copy link
Contributor

This conversation intrigues me. Sorry if I'm butting in.

I'll need to look into editorconfig a bit. My immediate concern is that its going to clash with #203 which is currently attempting to leverage StyleCop.

AFAIK, they serve different purposes:

  • editorconfig is a formatter, which means cleanup of format and spacing inconsistencies, among other things. It's usually applied before every commit, to ensure the commited code is all presented in the same way.
    The changes of a formatter can always be automatically applied. It can be run on the users' computers, because formatters are lightweight.
  • StyleCop is a linter, which means style warning or enforcement. It tries to give a common ground to the way things are programmed.
    And if I understood StyleCop correctly, it also does some static analysis to look after common programming errors.
    Linters are usually run in CI, because they often are heavy programs. Also, their changes are manually applied. This is because unlike formatters, the changes they suggest aren't trivial and need human intervention.

You can totally have both. They complement each other :)

@robkeim
Copy link
Contributor

robkeim commented Oct 18, 2017

Thanks @felix91gr for chiming in here! You're correct that StyleCop does static analysis although it can be run locally on every build as well as in the CI, particularly given our project isn't very large.

IMO, the discussion around linting/formatting has been a bit confused by two distinct things we're trying to enable:

  1. Static analysis/linting on the C# solution (generators and examples) to ensure that all of the contributors are following a standard format in order to make the code look clean and uniform
  2. Enable teaching moments for students by having their editor indicate they're not following C# coding guidelines/best practices

They're related problems with overlap but not the same. For example, forcing tabs instead of spaces (or vice versa) might be something interesting for us to do in the C# solution for the sake of uniformity but that's not something we want to force upon the students when they're building their solutions.

Another example would be having a rule to ensure dead code is removed. This could be something we agree we want to remove and have the build for the generators/examples solution fail is dead code is there. However, for students we'd want to let them proceed but give them a warning that dead code exists and help them understand the importance of that rule existing.

For the case of .editorconfig it seems like a good tool for the first issue but not great for the second.

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 a pull request may close this issue.

4 participants