+1 to all mentioned points above What about changes that refactor code to make it more readable & maintainable? My point of view is that this is important to keep the quality of a project and maintain the codebase. While some people think "if ain't broke it don't fix it". The reality is that software is in constant flux and needs to be cared for. If we are afraid to refactor or push back on it, means we should improve testing and coverage so this is not the case.
On Sat, Dec 16, 2017 at 8:46 AM, Naveen Swamy <[email protected]> wrote: > +1. > > I think the author should also allow maintainers to edit the files, so they > can make small changes if necessary(update docs, etc.,) > > https://help.github.com/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork/#enabling-repository-maintainer-permissions-on-existing-pull-requests > > > -Naveen > > On Fri, Dec 15, 2017 at 7:14 PM, pracheer gupta <[email protected]> > wrote: > >> +1 to all the 3 points (2 from Bhavin, 1 from Marco) >> >> > On Dec 15, 2017, at 5:37 PM, Markus Weimer <[email protected]> wrote: >> > >> > On Fri, Dec 15, 2017 at 5:00 PM, Bhavin Thaker <[email protected]> >> > 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 >>
