8000 added a tailored CodeFormatter to Octokit by shiftkey · Pull Request #807 · octokit/octokit.net · GitHub
[go: up one dir, main page]

Skip to content

added a tailored CodeFormatter to Octokit #807

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

Merged
merged 4 commits into from
Nov 4, 2015
Merged

added a tailored CodeFormatter to Octokit #807

merged 4 commits into from
Nov 4, 2015

Conversation

shiftkey
Copy link
Member

This is wired up in the build script so you can run it at your leisure:

.\build FormatCode

The source code for the tool: shiftkey/Octokit.CodeFormatter#1

I disabled a couple of rules from the upstream codeformatter repo which go against our coding style:

  • new line above first using statement
  • specify the visibility of a field/member explicitly
  • private fields naming style

The net result is that things are actually pretty good, and we get some cleanup of whitespace for free.

  • is the unicode (c) correct inside AssemblyInfo.cs?
  • [x] update AssemblyInfo.cs contents 👢ed
  • braces for empty constructors - what rule is this?
  • [ ] remove copyright rename rule 👢ed
  • wait for Obsolete assert ex.throws #799 to land and then rebase this
  • docs docs docs
  • hold for VS2015 support
  • run over codebase once support redirect requests natively #808 is merged

@khellang
Copy link
Contributor

Nice! 🔥 dat trailing whitespace! Maybe you should change the Copyright © Microsoft 2012 thingy while you're at it 😉

@@ -8,18 +8,20 @@ public class InterfaceNotFoundException : Exception
public InterfaceNotFoundException() { }

public InterfaceNotFoundException(string type)
: base(CreateMessage(type)) { }
: base(CreateMessage(type))
{ }

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks a bit odd to me 😯

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had a look into this, and it turns out the rule is working this way due to:

  • open brace always on a newline
  • close brace (where trivial) is moved on to the same line

If someone wants to fight me on this and put it back, they're more than welcome to poke around and see if they can fix this over on my CodeFormatter fork - it's BraceNewLineRule you want to look at...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aaaahhh...sorry, I must have forgotten to put on my 👓 when I looked over this...it was the indentation that got me...:nothingtoseehere: :movealong: 😳

@hnrkndrssn
Copy link
Contributor

Other than those {} being a few spaces short, I'm liking it 👍 👍 Will certainly make it easier to ensure that the codeformatting stays the same 😁

@shiftkey
Copy link
Member Author
tools\Octokit.CodeFormatter\tools\CodeFormatter.exe Octokit/Octokit.csproj /nocopyright
Octokit.csproj
ERROR: Type loading error detected. In order to run this tool you need either Visual Studio 2015 or Microsoft Build Tool
s 2015 tools installed.
- Could not load file or assembly 'Microsoft.Build, Version=14.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a'
or one of its dependencies. The system cannot find the file specified.

mutter mutter mutter

@shiftkey shiftkey changed the title [WIP] added a tailored CodeFormatter to Octokit added a tailored CodeFormatter to Octokit Nov 4, 2015
@shiftkey
Copy link
Member Author
shiftkey commented Nov 4, 2015

Reviving this now that we're on VS2015. Go AppVeyor Go!

shiftkey added a commit that referenced this pull request Nov 4, 2015
added a tailored CodeFormatter to Octokit
@shiftkey shiftkey merged commit f0111aa into master Nov 4, 2015
@shiftkey shiftkey deleted the codeformatter branch November 4, 2015 22:53
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 this pull request may close these issues.

3 participants
0