mxnet-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From kellen sunderland <kellen.sunderl...@gmail.com>
Subject RE: Apache MXNet Development Processes: Proposed update
Date Fri, 22 Dec 2017 08:36:20 GMT
Sheng’s comments are correct.  GitHub will save review comments for ‘outdated’ commits,
and they’re surfaced in the interface.  I frequently amend then force push (i.e. change
history) to address comments in order to keep my commits tidy.  It’s always possible to
expand a previous version selection to see the old comment history.

Example: https://github.com/awslabs/sockeye/pull/31

-Kellen

From: Zha, Sheng
Sent: Friday, December 22, 2017 12:34 AM
To: dev@mxnet.incubator.apache.org
Subject: Re: Apache MXNet Development Processes: Proposed update

About comment on b), I think after rebase the existing review comments will remain in the
PR even if rebase and history rewriting happens in the fork. The corresponding commit may
be gone because of the change in commit history in the PR fork and thus the link change.

Best regards,
-sz
On 12/15/17, 5:38 PM, "Markus Weimer" <markus@weimo.de> wrote:

    On Fri, Dec 15, 2017 at 5:00 PM, Bhavin Thaker <bhavinthaker@gmail.com>
    wrote:
    
    >    a) It is NOT recommended for a committer to merge pull requests that the
    >    committer authored. Instead the committer MUST get at least one approval
    >    from another committer to merge his/her pull request.
    >
    
    +1
    
    
    >    - b) When you update a pull request with upstream, you MUST use rebase
    >    to ensure that the pull request is easy to review by the community. See
    > the
    >    how-to link here:
    >    https://mxnet.incubator.apache.org/community/contribute.html
    
    
    Doesn't this potentially erase the review history on GitHub?
    
    Markus
    



Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message