AdaephonBen / cryptex

2 stars 0 forks source link

Refactor githooks #12

Open npalladium opened 4 years ago

RachitKeertiDas commented 4 years ago

Currently there's no action being taken on new .go files. I'd suggest running running gofmt -s -w . and then go build in the server directory, at the very least.

npalladium commented 4 years ago

"new" go files?

On Mon, Jun 1, 2020, 20:14 RachitKeertiDas notifications@github.com wrote:

Currently there's no action being taken on new .go files. I'd suggest running running gofmt -s -w . and then go build in the server directory, at the very least.

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/npalladium/cryptex/issues/12#issuecomment-636900073, or unsubscribe https://github.com/notifications/unsubscribe-auth/AINGXXOO75A7TBCFO4B6WADRUO5D3ANCNFSM4NOIHKQA .

RachitKeertiDas commented 4 years ago

Staged .go files

npalladium commented 4 years ago

I thought I'd been running go fmt and stuff. Feel free to fix.

tanmayr2 commented 4 years ago

Amazing, even I didn't notice gofmt was missing, figured the else block had it covered. However I don't see the point in doing a go build, is that necessary?

CrazyByDefault commented 4 years ago

No point building every commit, IMO. go fmt for sure as a pre-receive hook.

npalladium commented 4 years ago

go fmt on every commit please. Makes diffing easier.

But build we can do on some other criteria.

On Thu, Jun 4, 2020, 20:01 Shashank Varanasi notifications@github.com wrote:

No point building every commit, IMO. go fmt for sure as a pre-receive hook.

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/npalladium/cryptex/issues/12#issuecomment-638886312, or unsubscribe https://github.com/notifications/unsubscribe-auth/AINGXXMSSZWP2NGWXDYWBDTRU6V2XANCNFSM4NOIHKQA .

CrazyByDefault commented 4 years ago

Yeah, that's fair. pre-commit is better for our use case, good catch.

Builds need to happen when we tag commits, through CI/CD, so no githook will trigger a build ideally.

RachitKeertiDas commented 4 years ago

Okay. I have added gofmt in the pre-commit hook.

npalladium commented 4 years ago

Pre-commit hook for ESLint, prettier and gofmt along with a post-commit hook to support prettier have been added to master in 1a0908b.