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.

Sign up for GitHub

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
Closed

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

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions .github/workflows/auto-formatter.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
name: gofumpt

on:
push:
branches: [main]

jobs:
prettier:
runs-on: ubuntu-latest

steps:
- name: Checkout
uses: actions/checkout@v2
with:
ref: ${{ github.head_ref }}
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 •
edited
Loading

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 :)

with:
only_changed: true