Re: Review and update pull requests

2018-08-12 Thread Stamatis Zampetakis
I created for this purpose. You can have a look and let me know. 2018-08-12 3:08 GMT+03:00 Michael Mior : > Would someone be willing to put together a PR for the contributor > documentation? That way we have some concrete and specific changes to

Re: Review and update pull requests

2018-08-11 Thread Stamatis Zampetakis
I also believe that rebase and force-push makes the review process more difficult and that is why I raised the question in order to formalize the procedure. I think the email of Vladimir is quite complete towards that direction so it would be helpful to update the site accordingly. Concretely, I

Re: Review and update pull requests

2018-08-10 Thread Julian Hyde
I agree with most of what Vladimir said. But briefly: * It isn’t often necessary for the contributor to rebase. The reviewer will ask for a rebase if it’s really needed. * If you add a commit after an initial review, it’s really important that you do not squash (or amend). The reviewer wants to

Re: Review and update pull requests

2018-08-10 Thread Vladimir Sitnikov
Stamatis>Personally, I always perform rebase followed by a forced push I was inclined to use that policy in early days, yet I think it should not be the main way. Bellow assumes GitHub. If we happen to use Gerrit things might shine with a different colour. I suggest the following. FAQ: Q: I

Re: Review and update pull requests

2018-08-10 Thread Michael Mior
IMO, it depends on how extensive the changes are. Eventually we certainly prefer the rebase and force push approach . However, if the PR is complex and a lot of changes have been made, it can be helpful to temporarily add a commitment (which will be squashed later) to help the reviewer see what

Review and update pull requests

2018-08-10 Thread Stamatis Zampetakis
Hi, Contributing to Calcite is quite well documented in the website ( https://calcite.apache.org/develop/#contributing). One minor thing that might be missing is how to update the pull request when changes are required by the reviewers. Personally, I always perform rebase followed by a forced