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.

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

Use gödel to build and publish product #15

Merged
merged 1 commit into from
Dec 5, 2016

Conversation

nmiyake
Copy link
Contributor

@nmiyake nmiyake commented Dec 5, 2016

This change is Reviewable

@nmiyake nmiyake force-pushed the addGodel branch 3 times, most recently from ec1fef4 to bda556e Compare December 5, 2016 00:21
@uschi2000
Copy link
Contributor

The circle build passes although a test fails.

@uschi2000
Copy link
Contributor

Where did the imports of the form go get -u -v golang.org/x/tools/go/gcimporter15 go?

@nmiyake
Copy link
Contributor Author

nmiyake commented Dec 5, 2016

Yes the Circle thing I'm looking at now. It's actually an interesting case we've seen elsewhere, where go test doesn't report the failure, but the JUnit XML output does. The XML is actually correct -- right now the "main" test isn't working like it's supposed to -- since the function calls exec, that kills the test process so only 1 of the 3 tests in that suite run. go test doesn't recognize this as a failure, but the JUnit output does, and that's why the report has a failure.

Working on fixing the test more formally by making them integration tests (gödel has first-class support for doing this, and it makes sense in this case).

As for the imports, I'm guessing those were used by one of the other tools that was being fetched by go get since the gcimporter15 package isn't used by any of the core code in this project. Pretty much all of that code is included as part of the gödel executable.

@uschi2000
Copy link
Contributor

uschi2000 commented Dec 5, 2016 via email

@nmiyake nmiyake force-pushed the addGodel branch 2 times, most recently from a2ce998 to 13af80a Compare December 5, 2016 01:48
@nmiyake
Copy link
Contributor Author

nmiyake commented Dec 5, 2016

OK, this should be ready for review now.

@nmiyake
Copy link
Contributor Author

nmiyake commented Dec 5, 2016

Also, ran the publish task manually locally and verified that it properly creates and uploads the product to Bintray in the same way that the previous publish script did.

@uschi2000
Copy link
Contributor

OK, let's try this.

@uschi2000 uschi2000 merged commit 32b190e into palantir:develop Dec 5, 2016
@nmiyake nmiyake deleted the addGodel branch December 5, 2016 16:44
@nmiyake
Copy link
Contributor Author

nmiyake commented Dec 5, 2016

Great, looks like merge build completed successfully. Do you want to try to tag a minor version bump to ensure that publishing works as well, or would you rather wait for a real release?

@uschi2000
Copy link
Contributor

Tagged 1.1.1-rc1

@nmiyake
Copy link
Contributor Author

nmiyake commented Dec 5, 2016

Great, looks like publish succeeded!

Is there an easy way to validate that the artifact behaves as intended? If not I think we're still fine, but if there is might be worth trying it out just to verify that everything is okay. Otherwise, I think we're set!

@uschi2000
Copy link
Contributor

Tried in GJD, looks good.

@uschi2000
Copy link
Contributor

Thanks, Nick!

@nmiyake
Copy link
Contributor Author

nmiyake commented Dec 5, 2016

Great to hear -- np!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants