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

Add 2.3.0 to changelog.md & mention the breaking change in FacetFilter #600

Merged
merged 1 commit into from Jan 29, 2018

Conversation

a-magdy
Copy link
Contributor

@a-magdy a-magdy commented Jan 25, 2018

We upgraded from 2.1.0 to 2.3.0 and it took us a while to understand why the requests sent from searchkit to our backend look like this:

{
 "post_filter": {
  "term": {
   "undefined": "brands"
  }
 },
 "aggs": {
  "_type3": {
   "filter": {
    "term": {
     "undefined": "brands"
    }
   },
   "aggs": {
    "undefined": {
     "terms": {}
    },
    "undefined_count": {
     "cardinality": {}
    }
   }
  }
 }
}

I had to check the commits one by one, from 2.1.0 to get the to this change: d7f9eab#diff-f172a23508b6de00b38c007df825d91a

So I think it might save someone else's time to mention the breaking change in the change log file

thanks :)

@CLAassistant
Copy link

CLAassistant commented Jan 25, 2018

CLA assistant check
All committers have signed the CLA.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 98.69% when pulling 8a4c129 on Quadric:update-changelog into 9cd00cd on searchkit:develop.

@zvictor
Copy link

zvictor commented Jan 29, 2018

the commit message states that there is a breaking change on it, however the notice didn't go to the changelog. it would be a good practice to always add such messages to the changelog in the same commit that causes the break.

@ssetem

@ssetem
Copy link
Member

ssetem commented Jan 29, 2018

Thanks @zvictor , sorry it was not obvious and not in changelog

@ssetem ssetem merged commit ad87672 into searchkit:develop Jan 29, 2018
@zvictor zvictor deleted the update-changelog branch January 30, 2018 10:32
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.

None yet

5 participants