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

Jump to bottom

Add chamber-$(VERSION).sha256sums generation #126

Merged
merged 1 commit into from
Sep 5, 2018
Merged

Add chamber-$(VERSION).sha256sums generation #126

merged 1 commit into from
Sep 5, 2018

Conversation

nickatsegment
Copy link
Contributor

Fixes: #120

nickatsegment
nickatsegment commented Sep 4, 2018
View reviewed changes
Makefile
clean:
rm -rf ./dist

dist:
mkdir dist
GOOS=darwin GOARCH=amd64 CGO_ENABLED=0 go build $(LDFLAGS) -o dist/chamber-$(VERSION)-darwin-amd64
GOOS=linux GOARCH=amd64 CGO_ENABLED=0 go build $(LDFLAGS) -o dist/chamber-$(VERSION)-linux-amd64
@which sha256sum 2>&1 > /dev/null || ( \
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'm not especially proud of this platform-specific code. Improvements appreciated.

Copy link
Contributor

Choose a reason for hiding this comment

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

we could use shasum -a 256 on mac instead of requiring sha256sum. is that what you meant by platform-specific code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I was hoping to find some program that'd work cross platform. I've heard of shasum creating subtly incompatible sum files (maybe unfounded rumours), so it'd be nice to stick to one. I bet this shell code could be nicer maybe, and error earlier? I dunno. Good enough I guess.

systemizer
systemizer approved these changes Sep 5, 2018
View reviewed changes
Copy link
Contributor

@systemizer systemizer left a comment

Choose a reason for hiding this comment

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

This looks good to me. i think requiring sha256sum for those who are making distributions is a reasonable ask

@nickatsegment nickatsegment merged commit e743d3b into master Sep 5, 2018
@nickatsegment nickatsegment deleted the add-sha256sums branch September 5, 2018 17:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@systemizer systemizer systemizer approved these changes

@dfuentes dfuentes Awaiting requested review from dfuentes

@boggsboggs boggsboggs Awaiting requested review from boggsboggs

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@nickatsegment @systemizer