[GitHub] [pinot] wirybeaver commented on pull request #9710: [WIP] download offline segments from peers

[GitBox <gi...@apache.org>]

wirybeaver commented on PR #9710:
URL: https://github.com/apache/pinot/pull/9710#issuecomment-1317791484

   > > A newbie question, the [guide](https://docs.pinot.apache.org/developers/developers-and-contributors/contribution-guidelines#creating-a-pull-request-pr) recommend to use git rebase. AFAIK, all commits will be squash to a single commit and thus can guarantee that make the commit history linear when the PR got merged. If so, shall we use git merge when pull the remote master change? Otherwise, I need to force overriding every time push after git rebase. @jadami10
   > 
   > With github, I generally use `git merge` when updating PRs because it keeps the change history. When you rebase/force push, the reviewer loses the "see changes since your last review" button. It's not a big deal, just a quality of life change.
   > 
   > If you've ever used something like phabricator, it does a better job of decoupling changes from their actual commits, so even if someone rebases/force pushes, it can show you just the changes that user made.
   
   Thanks. Totally agree with you I use github before joining uber. I am meant to use `get merge` but the official Pinot OSS developer guide uses `git rebase` at the bottom when pushing to the remote branch, which made me confused. We need to update the instruction. https://github.com/apache/pinot/pull/9710


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org