Skip to content

Add Github action that checks formatting.#1019

Merged
maciejwalkowiak merged 2 commits intomainfrom
spotless-gh-action
Nov 2, 2020
Merged

Add Github action that checks formatting.#1019
maciejwalkowiak merged 2 commits intomainfrom
spotless-gh-action

Conversation

@maciejwalkowiak
Copy link
Copy Markdown
Contributor

📢 Type of change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring

📜 Description

Adds Github action to check if code is properly formatted.

💡 Motivation and Context

We often get PRs merged that do not have code properly formatted. As the result, formatting fixes sneak in to other PRs adding noise.

This change will prevent PR build success in case code is not properly formatted.

💚 How did you test it?

📝 Checklist

  • I reviewed submitted code
  • I added tests to verify changes
  • All tests passing
  • No breaking changes

@codecov-io
Copy link
Copy Markdown

codecov-io commented Oct 29, 2020

Codecov Report

Merging #1019 into main will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##               main    #1019   +/-   ##
=========================================
  Coverage     71.97%   71.97%           
  Complexity     1320     1320           
=========================================
  Files           135      135           
  Lines          4813     4813           
  Branches        491      491           
=========================================
  Hits           3464     3464           
  Misses         1091     1091           
  Partials        258      258           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7618ab2...d0c8e59. Read the comment docs.

Copy link
Copy Markdown
Member

@bruno-garcia bruno-garcia left a comment

Choose a reason for hiding this comment

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

Does it push the diff back into the PR?

What about something like this: https://github.com/getsentry/sentry-cocoa/blob/master/.github/workflows/format-code.yml

@marandaneto
Copy link
Copy Markdown
Contributor

Does it push the diff back into the PR?

What about something like this: https://github.com/getsentry/sentry-cocoa/blob/master/.github/workflows/format-code.yml

I'd say it doesn't, and now I wonder what's the difference between this PR and this:

https://github.com/getsentry/sentry-java/blob/main/build.gradle.kts#L129-L131

because they do pretty much the same thing and the CI already calls gradle build

@maciejwalkowiak
Copy link
Copy Markdown
Contributor Author

Does it push the diff back into the PR?
What about something like this: https://github.com/getsentry/sentry-cocoa/blob/master/.github/workflows/format-code.yml

I'd say it doesn't, and now I wonder what's the difference between this PR and this:

It doesn't but I can add this action that commit changes back.

https://github.com/getsentry/sentry-java/blob/main/build.gradle.kts#L129-L131

because they do pretty much the same thing and the CI already calls gradle build

The difference is that calling gradle build only applies formatting configuration and with this PR the build will fail if code is not formatted.

@marandaneto
Copy link
Copy Markdown
Contributor

Does it push the diff back into the PR?
What about something like this: https://github.com/getsentry/sentry-cocoa/blob/master/.github/workflows/format-code.yml

I'd say it doesn't, and now I wonder what's the difference between this PR and this:

It doesn't but I can add this action that commit changes back.

https://github.com/getsentry/sentry-java/blob/main/build.gradle.kts#L129-L131
because they do pretty much the same thing and the CI already calls gradle build

The difference is that calling gradle build only applies formatting configuration and with this PR the build will fail if code is not formatted.

I see, makes sense, I have this as a pre-commit hook actually.
https://github.com/getsentry/sentry-java/blob/main/hooks/pre-commit

but yeah CI would be great as well

@marandaneto marandaneto mentioned this pull request Oct 30, 2020
@maciejwalkowiak maciejwalkowiak merged commit c59f373 into main Nov 2, 2020
@maciejwalkowiak maciejwalkowiak deleted the spotless-gh-action branch November 2, 2020 09:29
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