Static assets now built into the binary by sporkmonger · Pull Request #63

Static assets now built into the binary by sporkmonger · Pull Request #63 · buzzfeed/sso

		fsHandler, err := loadFSHandler()
		if err != nil {
		logger.Fatal(err)
		os.Exit(1)

Choose a reason for hiding this comment

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

Actually, I meant to check whether .Fatal calls os.Exit or not before committing. Looks like it does, so this line isn't needed.

Choose a reason for hiding this comment

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

That said this is one of those "this should never happen" errors.

I opted to check internal/auth/statik/statik.go into source control so that you wouldn't need to call go generate immediately after a clone to get the project to compile and because I suspect it won't change often. But I could also easily see the argument in the other direction since it's a generated file.

So it seems like the gpm install in the earlier build step maybe isn't doing what I'm expecting it to?

Edit
Narrator: It wasn't actually gpm install.

OK, closer, but maybe an issue with wrong version of Go?

Failures that only happen in CI/CD are always fun.

I'm going to squash the fighting-with-CircleCI commits.

Thanks for opening @sporkmonger! We'll be working on reviewing and getting this merged as soon as we can.

One more request - we should be able to also get rid of copying over the static directory in the Dockerfile when building the docker image.

Since we are still working on figuring out a good way to build and push docker images for forked community contributions, I've pushed a copy of your pr branch on buzzfeed/sso (https://github.com/buzzfeed/sso/tree/pr/63) to validate on our internal infrastructure so that we can get this merged!

Thanks for making those changes @sporkmonger, this looks good to me, can you please squash the commits and I'll approve and merge!