twingly / twingly-search-api-java

:coffee: Twingly Blog Search API in Java
https://developer.twingly.com/resources/search/
3 stars 7 forks source link

Introduced QueryBuilder approach #11

Closed xSAVIKx closed 8 years ago

xSAVIKx commented 8 years ago

Added QueryBuilder for easier query creation. Query is now just POJO and all functionality is now in Client impl. Brought back @Unroll test for query string building. added support for .properties file, that is now used by both gradle and Java for getting current versions.

walro commented 8 years ago

Hi @xSAVIKx,

We'll need you to re-submit the PR, I think you forgot to rebase with our master before you did your changes. You have duplicated commits and our README changes are gone.

I think you could create a branch on your repository with your current master's state and then rewind your master to the commit before your changes. Rebase your master with our's and then cherry pick your commits from your temporary branch to your master and then re-submit the PR. Or, to make it even better, you can create a branch which you submit the PR from to keep your master clean.

xSAVIKx commented 8 years ago

You asked to make rebase - I've done rebase, so all my new changes have overwritten your changes (cause there was refactoring in readme). And there are duplicated commits due to rebase.

xSAVIKx commented 8 years ago

And your changes are in place, except URLs in Readme (check .travis.yml, for example)

walro commented 8 years ago

Ok, I think terminology is the problem here, I meant that your fork was out of sync with our master before you started with your work.

walro commented 8 years ago

When I said rebase I meant sync like this: https://robots.thoughtbot.com/keeping-a-github-fork-updated

xSAVIKx commented 8 years ago

Yeap, I've done that. I've done rebase on your repo.

walro commented 8 years ago

Ok, let's stop focusing on the word rebase and lets focus on the problem we're seeing now. :) You did changes to the README before synching our latest changes. All of our changes in the README are now gone.

If we step back a bit, not doing any rebase/resync/whatever-you-wanna-call it would lead to a likely merge conflict situation when submitting (rather, merging) the PR. So what we want is a PR that is based on our current master in order for us to effortlessly review and merge.

xSAVIKx commented 8 years ago

OK :-)