Skip to content

Conversation

@cjost1988
Copy link
Contributor

  • Added Aggregation\Terms::setOrders
  • Added test for Aggregation\Terms::setOrders
  • Linter fixed another comment

If you want me to remove the linter fix, I'll can remove it. But I just followed the instructions and the linter changed the line. And since it was just one small change I took it into my PR.

@ruflin ruflin merged commit 7acab37 into ruflin:master Apr 6, 2018
@ruflin
Copy link
Owner

ruflin commented Apr 6, 2018

@cjost1988 Thank you for the high quality PR. Changelog, linting, tests - everything there ❤️

@cjost1988
Copy link
Contributor Author

lol I just wanted to add the pull request link in the changelog :D

Thank you for your quick process. Glad to help improving.
And in the end we all benefit from good quality ;)

@ruflin
Copy link
Owner

ruflin commented Apr 6, 2018

haha, missed that one. Feel free to open an other PR, otherwise will fix it on release day when going through the changelog.

@cjost1988
Copy link
Contributor Author

@ruflin #1481 is containing the fix for the changelog.

Thanks :)

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