bitemyapp / bloodhound

Haskell Elasticsearch client and query DSL
bitemyapp.com
BSD 3-Clause "New" or "Revised" License
422 stars 118 forks source link

Master zoomin #248

Closed renahkws closed 5 years ago

renahkws commented 5 years ago

Hi, we make heavy use of Elastic with Bloodhound at Zoominsoftware and we've been maintaining a fork for a while and adding features as we need them. We recently merged your latest and greatest into our fork and now would like to submit our changes to be merged into the main for others to benefit from. We've followed the Elastic docs pretty carefully, and are successfully using most of these changes in our prod environments. Let us know what you think. The updates include the following:

bitemyapp commented 5 years ago

Build's green on our end: https://travis-ci.org/bitemyapp/bloodhound/builds/397789694

But it looks like some of the test suites don't build or are failing with this PR. Before we spend time reviewing this large passel of changes, are you interested in fixing the build?

renahkws commented 5 years ago

Can you advise the best way to review the test suites? I tried running stack test a number of times, and each time it just ate up most of the CPU and disk for 20-30 minutes with no sign of finishing.

On Tue, Dec 18, 2018 at 9:30 PM Chris Allen notifications@github.com wrote:

Build's green on our end: https://travis-ci.org/bitemyapp/bloodhound/builds/397789694

But it looks like some of the test suites don't build or are failing with this PR. Before we spend time reviewing this large passel of changes, are you interested in fixing the build?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/bitemyapp/bloodhound/pull/248#issuecomment-448341293, or mute the thread https://github.com/notifications/unsubscribe-auth/AdqIeHcN-7wM_JssfJzM7uufxBbVAI8fks5u6UJpgaJpZM4ZYWLS .

bitemyapp commented 5 years ago

Can you advise the best way to review the test suites?

Pretty sure the Makefile has stuff in it. Failing that, you can see that the build failed in TravisCI. If you cannot get the test suite working locally you are welcome to spin this PR to keep tipping TravisCI over until you figure out why it's not working.

bitemyapp commented 5 years ago

@renahkws this PR changes too much and there's too much sustained brokenness for me to take the risk of merging this. I don't want to mislead you or misdirect your efforts so please don't do any further work on this for the sake of an upstream merge.

I'm closing this PR. If you'd like to get this into master then I would like to see individual, smaller changes justified and documented with passing tests that cover the new changes.