Skip to content

Enforce code style with dotnet format #532

Merged
gregkalapos merged 5 commits intoelastic:masterfrom
gregkalapos:DotNetFormat
Oct 12, 2019
Merged

Enforce code style with dotnet format #532
gregkalapos merged 5 commits intoelastic:masterfrom
gregkalapos:DotNetFormat

Conversation

@gregkalapos
Copy link
Copy Markdown
Contributor

@gregkalapos gregkalapos commented Oct 3, 2019

Addressing #510.

What's new:

With dotnet format --dry-run --check we make sure all our C# code is formatted the way it should be formatted according to our .editorconfig file. If that is not the case then dotnet format returns with non-zero exit code.

I added this step only to the Linux build - I think that's enough, no need to run it on multiple OSs. Let me know if there is a better place for it.

Next step:

This build currently fails because we have code style violations - those are addressed in #535 (opened a separate PR, otherwise it'd be hard to review this), so that'll be merged first. #536 shows the status after #535 is merged into that branch: Update: code cleanup done, this PR branch should be also ok.

Screen Shot 2019-10-04 at 12 52 40

@gregkalapos gregkalapos self-assigned this Oct 3, 2019
@gregkalapos gregkalapos marked this pull request as ready for review October 4, 2019 10:42
Copy link
Copy Markdown
Contributor

@kuisathaverat kuisathaverat left a comment

Choose a reason for hiding this comment

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

when you remove the tab from the GitHub notification LGTM :)

Copy link
Copy Markdown
Member

@Mpdreamz Mpdreamz left a comment

Choose a reason for hiding this comment

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

This looks good to me.

It would be good for .ci to start moving to local .NET tools when this repository moves over to .NET Core 3.0 so that the build stays repeatable. cc @kuisathaverat

@kuisathaverat
Copy link
Copy Markdown
Contributor

kuisathaverat commented Oct 10, 2019

It would be good for .ci to start moving to local .NET tools when this repository moves over to .NET Core 3.0 so that the build stays repeatable. cc @kuisathaverat

probably we'd have to have two builds, one for .NET 2.x, and another for .NET 3.x to ensure compatibility

@gregkalapos
Copy link
Copy Markdown
Contributor Author

It would be good for .ci to start moving to local .NET tools when this repository moves over to .NET Core 3.0 so that the build stays repeatable. cc @kuisathaverat

probably we'd have to have two builds, one for .NET 2.x, and another for .NET 3.x to ensure compatibility

For compatibility actually we'll also need some 2.x and 3.x projects that use the agent. Now I'm not totally sure, but I think if we have 2.x and 3.x projects that test the agent then 1 build is enough, because that'd pull the corresponding .NET Core version anyway. I opened #546

gregkalapos added a commit that referenced this pull request Oct 12, 2019
The `.editorconfig` was changed in #497. That PR was about "Central Config Implementation". Let's revert the `.editorconfig` change and in case we really want to change it please do it in a separate PR.
gregkalapos and others added 5 commits October 12, 2019 13:49
Co-Authored-By: Ivan Fernandez Calvo <kuisathaverat@users.noreply.github.com>
Co-Authored-By: Ivan Fernandez Calvo <kuisathaverat@users.noreply.github.com>
@gregkalapos gregkalapos merged commit 230f774 into elastic:master Oct 12, 2019
@gregkalapos gregkalapos deleted the DotNetFormat branch October 12, 2019 12:25
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.

4 participants