Skip to content
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

Chore: Added Github Action to auto format code on push to main #187

Closed
wants to merge 1 commit into from

Conversation

jay-dee7
Copy link
Contributor

This PR adds a Github Action to auto-format code using gofumpt on push to master.

Fixes #93

Signed-off-by: jay-dee7 jasdeepsingh.uppal@gmail.com

Signed-off-by: jay-dee7 <jasdeepsingh.uppal@gmail.com>
fetch-depth: 0

- name: Auto Format code
uses: iamnotaturtle/auto-gofmt@v2.0.0
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the PR, but I'd prefer not introducing a 3rd party github action for formatting.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey @mfridman does that mean the regular goimports or go fmt would be preferred here?

Copy link
Member

@mfridman mfridman Apr 15, 2022

Choose a reason for hiding this comment

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

Ye, most projects I've seen have linting, formatting, etc. run against the pull request itself. If there is a diff or something doesn't match it is up to the author to fix (CI fails).

I don't have a strong opinion on whether to use gofmt or gofumpt. The latter has pre-built binaries so can curl it.

Something like this can be added before the tests:

- name: Check Go code formatting
  run: |
    if [ "$(gofmt -s -l . | wc -l)" -gt 0 ]; then
      gofmt -s -l .
      echo "Please format Go code by running: go fmt ./..."
      exit 1
    fi

This avoids a dependency on a GitHub Action and enforces standard PR's to make reviews easier. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the CI failing actually is the way go. I have it failing on all of my other projects. Usually author is responsible to fix them. I'll go ahead and make the PR changes :)

@oxisto
Copy link
Collaborator

oxisto commented May 28, 2022

Closed in favour of #206

@oxisto oxisto closed this May 28, 2022
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.

Add go fmt check on github actions
3 participants