Formatting/convention config changes #134
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- The .gitattributes updates will hopefully help us handle our line endings in a consistent fashion no matter what the contributor's local setup is. - Some of the .gitattributes updates will help with diffs on the cmdline and merges too. - The .editorconfig should help keep formatting consistent, so we don't have to catch as much in code review and things are super reasonable and look as expected. - The .editorconfig still requires that the dev use the right IDE or plugin, if available, but it's better than nothing. - A lot of the .editorconfig settings are only available in Visual Studio after 2017 15.3, but that's what's necessary to build the multi-target netstandard2.0+netfx4.5 project and produce the .nupkg anyway. - Some of the .editorconfig settings are not really documented, but I imagine between Roslyn and CoreCLR they probably know what's up. - We'll want to decide if the `utf-8-bom` charset setting is the right thing to do for code files...We could also prevent VS from adding it for files that can't have a BOM, like hash-bang scripts or docs that are embedded like HTML partials and so forth. The Roslyn codebase uses `utf-8-bom` for some reason. I suppose if people edit those files on Windows and don't have the editorconfig plugin or the right version of their IDE, they might add the BOM and end up seeing silly merge conflicts. Not sure that's a good enough reason to put the BOM on some files and not others.
- Most people not using Visual Studio will be on Linux/Mac which won't want to add a BOM anyway (not sure what VS Code or Visual Studio on Mac do) - If on Windows and contributing, you're probably building with VS 15.3...at least if you intend to create the .nupkg with the multi-targetted project, and 15.3 has the right editorconfig handling built in
…les: - This doesn't apply all conventions, since some are just suggestions, and some only apply to new code. - Lots of BOMs are gone...we'll see if that was a good idea. - Some of these settings might not be the best, or need tweaking, but at least things will get to be more standardized moving forward.
masojus
commented
Nov 11, 2017
| var timeframe = obj as QueryRelativeTimeframe; | ||
| return timeframe != null && | ||
| _value == timeframe._value; | ||
| //var timeframe = obj as QueryRelativeTimeframe; |
There was a problem hiding this comment.
I need to remove the commented-out code.
baumatron
approved these changes
Nov 12, 2017
| ############################################################################### | ||
| #*.cs diff=csharp | ||
| *.cs text diff=csharp |
There was a problem hiding this comment.
What's diff=csharp all about?
There was a problem hiding this comment.
The diff attribute sets a parser/regexp for deciding what things to include and how to format the "hunk header" which shows up when you diff on the cmd-line. More info at "Defining a custom hunk-header" and at the gitattributes docs. There are a bunch of built-in patterns, but you can also define any arbitrary pattern if you wish.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add an .editorconfig file and .gitattributes so that going forward things are a little more standardized.
I'll change the base branch to 'master' once 'jm_UpdateCachedDatasetsBranch' is merged.