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

Fixing another bug related to builds without Grackle #110

Merged
merged 5 commits into from
Jun 2, 2021

Conversation

mabruzzo
Copy link
Contributor

@mabruzzo mabruzzo commented May 27, 2021

PR #49 introduced a minor change that breaks builds without Grackle (again).

I've fixed this locally (it's a really easy fix). But I'm taking this as an opportunity to try to setup a CircleCi test to verify that Enzo-E can be built without Grackle (Our other tests already take a while, so I'm not going to run any tests with this build; I'm just checking if the build succeeds).

I'm intentionally going to make some tests fail while testing this, but this will be ready for review by tomorrow. (I'll change this from WIP once it's ready and I'll add an edit to this post confirming that it's ready).

EDIT: I'm done. This is now ready for review.

@jwise77
Copy link
Contributor

jwise77 commented May 27, 2021

Looks good so far! I'll approve it when you finish with the testing and change it from WIP.

This attempt tries to build on the existing CI commands
@mabruzzo
Copy link
Contributor Author

This is now ready for review.

If there were ever a PR that should be squashed while merging, this would be the one.

@mabruzzo mabruzzo added the bug Something isn't working label May 27, 2021
@bwoshea
Copy link
Contributor

bwoshea commented May 30, 2021

@mabruzzo looks good to me! @jwise77 when you get a minute, could you look at this PR?

Copy link
Contributor

@jwise77 jwise77 left a comment

Choose a reason for hiding this comment

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

Looks good. I only found a typo in the circleci description that needs addressing. We can then squash-n-merge!

.circleci/config.yml Outdated Show resolved Hide resolved
.circleci/config.yml Show resolved Hide resolved
.circleci/config.yml Outdated Show resolved Hide resolved
@jwise77
Copy link
Contributor

jwise77 commented Jun 2, 2021

@bwoshea approved in his comment but didn't hit the approve button. That's 2 approvals and I'm squashing and merging!

@jwise77 jwise77 merged commit 6b0f17c into enzo-project:main Jun 2, 2021
@mabruzzo mabruzzo deleted the no-grackle_bugfix branch June 2, 2021 17:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants