Skip to content
This repository was archived by the owner on Jan 5, 2023. It is now read-only.

Make the gofmt CI test actually fatal#254

Merged
max-schaefer merged 1 commit intogithub:masterfrom
smowton:smowton/admin/fix-go-autoformat
Jul 10, 2020
Merged

Make the gofmt CI test actually fatal#254
max-schaefer merged 1 commit intogithub:masterfrom
smowton:smowton/admin/fix-go-autoformat

Conversation

@smowton
Copy link
Copy Markdown
Contributor

@smowton smowton commented Jul 9, 2020

Turns out gofmt doesn't actually return 1 when it finds problems, only when it finds source files which don't compile (all of which are now excluded).

This also fixes existing overlooked inconsistencies as a result of this mistake.

@smowton smowton requested a review from a team July 9, 2020 16:48
Copy link
Copy Markdown
Contributor

@max-schaefer max-schaefer left a comment

Choose a reason for hiding this comment

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

Thanks for catching that. Not as pretty, but if it works that's OK.

Since the clever logic for using environment variables to control whether formatting is only checked or actually enforced no longer works, could you just remove those variables?

Turns out gofmt doesn't actually return 1 when it finds problems, only when it finds source files which don't compile (all of which are now excluded).

This also fixes existing overlooked inconsistencies as a result of this mistake.
@smowton smowton force-pushed the smowton/admin/fix-go-autoformat branch from 2dce532 to d05657d Compare July 10, 2020 10:02
@smowton
Copy link
Copy Markdown
Contributor Author

smowton commented Jul 10, 2020

@max-schaefer done

@max-schaefer max-schaefer merged commit 9347413 into github:master Jul 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants