+1 (non-binding) to all points. On Dec 18, 2017 11:37 AM, "Pedro Larroy" <pedro.larroy.li...@gmail.com> wrote:
> +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 <mnnav...@gmail.com> 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 < > pracheer_gu...@hotmail.com> > > wrote: > > > >> +1 to all the 3 points (2 from Bhavin, 1 from Marco) > >> > >> > On Dec 15, 2017, at 5:37 PM, Markus Weimer <mar...@weimo.de> wrote: > >> > > >> > On Fri, Dec 15, 2017 at 5:00 PM, Bhavin Thaker < > bhavintha...@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 > >> >
