-
Notifications
You must be signed in to change notification settings - Fork 749
Format and White-spacing #205
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
Conversation
@tonyroberts any idea why |
As I side note, i don't fully agree with VS's choice of style, but I'd rather see consistent formatting. |
I'm fine with cleaning up whitespace etc, but I think reorganising the file structure should be done in a different PR. In particular, I don't think moving very old and out dated documentation into the root folder is a good idea. I think it would be better if it was updated and then moved - rather than delay these whitespace changes until that's done, dropping them from this PR seems sensible. |
Also the wording of commit f72c7e0 comes across as a bit derogatory towards mono (even if that wasn't the intention) and could give someone casually looking through the commit history the impression that mono is less supported than the microsoft runtime, which is not the case. |
@vmuriart hmm not sure what happened to the appveyor CI - looks like there was some problem with the github settings, which is odd as I'm pretty sure I didn't change them! Anyway, hopefully it will be working again now.. |
b67d068
to
ffe376b
Compare
@tonyroberts I was having similar thoughts about the file cleanup after i submitted the Good catch on the mono commit. I meant that in a joking manner (monkey-droppings) with the intention of removing their old format files since they now support the same files as vs. Looks like travis is still the only one running, atleast on this PR. I had a similar issue on my fork though (Travis being the problem in my case). To fix it I had to remove the hook for it, and re-add it from github. |
@tonyroberts what are your thoughts on |
@vmuriart cool, I'll merge this now and take a look at why the commit hooks aren't working later. I'll try just recreating it as you suggest. Thanks for doing this! |
@vmuriart wrt ff, it really depends on the situation. For simple changes I prefer to ff where possible, but when adding significant new feature or a larger set of changes I prefer to use a feature branch and keep the merge commit - so sounds like we're thinking along the same lines :) |
@vmuriart @tonyroberts the indent(ation) style for curly braces is a mess even after merge. Here are 2 contradictory examples: Allman style: K&R style: |
Good catch. Not sure why the linter's didn't pick it up. They should all be defaulting the K&R style since that's VS's default. On Mon, Apr 18, 2016 at 10:50 AM, denfromufa notifications@github.com
|
Ah, just realized whats going on. On Mon, Apr 18, 2016 at 11:32 AM, Victor Uriarte vmuriart@gmail.com wrote:
|
Didn't realize that there were only 2 |
@vmuriart @tonyroberts I think this PR should include one https://github.com/OmniSharp/omnisharp-roslyn/blob/dev/.editorconfig |
I forgot to add the |
To address #184.
Settled on using VS standard formatting for
.cs
and Python pep8 for.py
.Did some house cleaning and removed/integrated old docs, solution files, etc.
Should hopefully help solve constant whitespacing issue on pull requests.
Note, other than white-spacing and formatting nothing in the code has been changed.