You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@thrift.apache.org by GitBox <gi...@apache.org> on 2022/04/05 03:55:38 UTC

[GitHub] [thrift] Jimexist commented on pull request #2525: THRIFT-5521: [java gen] use jdk8 option type in java generator code

Jimexist commented on PR #2525:
URL: https://github.com/apache/thrift/pull/2525#issuecomment-1088240953

   > FWIW, I would strongly advise against rebasing as a mechanism for triggering CI builds. Rebasing makes it difficult for reviewers to see diffs with new changes to pull requests they've already reviewed, as it resets the changeset. It's very frustrating, especially for large pull requests.
   > 
   > As an alternative, committers can re-trigger the Travis and Mergeable checks using the "Checks" tab in the Pull Request. Appveyor builds can be re-triggered using Appveyor's dashboard (though, I think Appveyor should probably be replaced with GitHub Actions Windows builds).
   > 
   > Contributors can re-trigger builds by adding a new empty commit to their branch instead of rebasing. These empty commits do not need to be merged... they can be squashed out when the PR is accepted. GitHub's interface has a "Squash and merge" option that would do this, as well as let the committer doing the merge polish the commit message style. Of course, this could also be done on the command-line.
   
   @ctubbsii that's a fair point - and I wasn't aware of such issues before so thanks for pointing out.
   
   My reasoning for using rebase based approach for managing pull requests - (and I realize that such arguments can easily slide into a bikeshedding so just to be clear i'll be willing to accommodate either way of working as long as it's clearly mandated or preferred, just sharing here for knowledge):
   1. it's clearer than say merging master back to the pull request branch, as there's no merge commit, and you always know that you are diffing against latest master
   2. GitHub actually helps reviewers manage their review history - the "viewed" toggle button on the top right is something (afaik) that can persist across rebases - so no longer do you need to keep track of what's reviewed and what's changed - with that said, i acknowledge that GitHub isn't something that everyone uses for reviewing the code and i guess in this case i'll refrain from rebasing later


-- 
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: notifications-unsubscribe@thrift.apache.org

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