search

nishanths / license

Command line license text generator.
MIT License
663 stars 82 forks source link

Suggest `go fmt` changes instead of commiting to PR #45

Closed norwd closed 2 years ago

norwd commented 2 years ago

Run go fmt -x on pull requests, but only suggest theme instead of committing directly to the PR.

Closes #43

norwd commented 2 years ago

Instead of commiting the changes directly, this workflow now makes comments on the PR itself and suggests adding the changes. Allowing maintainers to opt-in to formatting changes.

image

Changes can be cherry-picked, or committed in batches through the GitHub UI.

image

See example: https://github.com/norwd/pword/pull/29/files#diff-2873f79a86c0d8b3335cd7731b0ecf7dd4301eb19a82ef7a1cba7589b5252261

nishanths commented 2 years ago

Thank you. This is much better than the check we have in master. But it may add a lot of formatting-related review comments, which isn't the main focus in a code review.

I feel we should do something like the following for this step. 

run: |
  out="$(gofmt -d . )"
  if [ -n "$out" ]; then
      echo "$out" && exit 1
  fi

It runs gofmt. If a diff is produced (i.e. formatting on the branch differs from gofmt), then it prints the diff and exits with an error.

nishanths commented 2 years ago

I made this pull request based on my comment above: https://github.com/nishanths/license/pull/46.

nishanths commented 2 years ago

Thank you for the change. But I will close this PR and use the other one.