> -1 for a merge commit. The feature branch was a good way to break down the > work and review it in chunks, but I think we still need to review the final > patch that will go in. That's why I asked for a PR for this. I never thought > that the branch would be merged without a final review, and that's a good > time to take care of rebasing or merging into this branch and then squashing.
If I understand correctly, you have two separate concerns: - Merging the feature branch without reviewing the whole change. - Actually using a merge commit instead of squashing the individual commits into a single one. Regarding the first one, I think feature branches in general would be the most useful if we reviewed them continuously on the feature branch itself and limit the review of the merge to the commit resolving the merge conflicts. Developers of a branch may put months of effort into it. If only direct commits into the main branch are taken seriously enough for immediate review, people could come to the (justified) conclusion that the best way for their work to not get lost is by developing on the main branch and not in a feature branch. Regarding the latter issue, I think a feature large enough for a separate feature branch is complex enough so that the more detailed history that a proper merge commit provides outweighs the disadvantage of a slightly more complicated history. Personally when I try to understand the motivation behind the existence of certain code lines I look up the commit that added them and read the whole commit. Naturally, the smaller these commits are, the easier it is to this. Let's discuss this further on the next Parquet sync and update this thread with the outcome. [ Full content available at: https://github.com/apache/parquet-mr/pull/527 ] This message was relayed via gitbox.apache.org for [email protected]
