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.

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

Update release strategy and migrate to circle 2 #355

Merged
merged 5 commits into from Sep 5, 2018

Conversation

Projects
None yet
2 participants
@ale7714
Copy link
Member

ale7714 commented Sep 4, 2018

  • OS X binary is compiled natively. This allows the test-reporter to use the system network libraries including the DNS resolver.
  • We're not releasing 2 Linux binaries:
    1. One with CGO disabled (which is how our current binary behaves)
    2. One with CGO enabled and that forces the use of net CGO for network call including
      DNS resolution
  • It migrates to circle 2.0

ale7714 and others added some commits Aug 29, 2018

Update building strategies
* We will build natively OS X binaries so it uses
  the system  DNS resolver
* For Linux, we will compile 2 binaries:
  * One with CGO disabled (which is how our current binary
    behaves)
  * One with CGO enabled and that forces the use of CGO for
    DNS resolution

@ale7714 ale7714 requested a review from wfleming Sep 4, 2018

@wfleming
Copy link
Member

wfleming left a comment

Mostly looks good, a few small points.

Note that after merging you'll want to edit protected branches on this repo to change which statuses are required.

Makefile Outdated
@@ -10,7 +10,7 @@ MAN_FILES = $(wildcard man/*.md)
MAN_PAGES = $(patsubst man/%.md,man/%,$(MAN_FILES))

PROJECT = github.com/codeclimate/test-reporter
VERSION ?= 0.6.2
VERSION ?= netcgo

This comment has been minimized.

@wfleming

wfleming Sep 4, 2018

Member

Is this really the default VERSION we want set all the time?

This comment has been minimized.

@ale7714

ale7714 Sep 4, 2018

Author Member

no sorry that was an accident

Makefile Outdated
build:
go build -v ${LDFLAGS} -o $(PREFIX)bin/test-reporter$(BINARY_SUFFIX)
go build -v ${LDFLAGS} -tags ${BUILD_TAGS} -o $(PREFIX)bin/test-reporter$(BINARY_SUFFIX)


build-all:

This comment has been minimized.

@wfleming

wfleming Sep 4, 2018

Member

I question the usefulness of keeping this rule in its current form: it's declaring it will build both linux & OS X binaries, but neither of the rules being executed involves any cross-compilation trickery anymore, so it's not actually possible for a single rule to compile both Linux & Max AFAICT.

@ale7714 ale7714 force-pushed the ap/netcg0 branch 4 times, most recently from 05bf35a to 72aafb4 Sep 4, 2018

@ale7714 ale7714 force-pushed the ap/netcg0 branch from 72aafb4 to 131edcf Sep 4, 2018

@ale7714 ale7714 force-pushed the ap/netcg0 branch from 910e414 to 2765da4 Sep 4, 2018

@ale7714

This comment has been minimized.

Copy link
Member Author

ale7714 commented Sep 4, 2018

@wfleming PR updated and test-reporter version bumped with changelog note.

@wfleming
Copy link
Member

wfleming left a comment

LGTM

Not totally necessary, but if we wanted to be a bit paranoid we could add more jobs to the CI pipeline to also exercise the cgo variant of the linux binary.

@ale7714 ale7714 merged commit 91112eb into master Sep 5, 2018

3 checks passed

ci/circleci: test_linux Your tests passed on CircleCI!
Details
ci/circleci: test_macos Your tests passed on CircleCI!
Details
license/cla Contributor License Agreement is signed.
Details

@ale7714 ale7714 deleted the ap/netcg0 branch Sep 5, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment