mxnet-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Anton Chernov <mecher...@gmail.com>
Subject Re: Feature branches for ARM and Android
Date Thu, 14 Jun 2018 10:12:25 GMT
Thank you Thomas for your suggestion, we already did exactly that and now
even have a CI verification for PR's to this branches in a public fork. The
main problem mentioned by Pedro already is that the changes we are doing
are already big and they are not going to be smaller over time. The merge
back to origin is going to be not only full of conflicts, but also
challenging to review.

As well such a PR could not be named starting with [MXNET-xxx] mentioning a
certain JIRA ticket, since it incorporates a batch of things tightly bound
to each other. It will be a big not separable list of tickets and changes
both specific to the problem and general improvements necessary to be made.
This is impossible to cherry-pick or revert separately and a completely
different branch needs to be maintained for release changes, general
improvements and specific task development.

Some general improvements require such amount of work (for example some
cmake improvements) that the initial issue becomes not solvable anymore in
a reasonable amount of time, burying both the potentially added value and
WIP improvements.

In general I don't understand the reason for such hard blocking of
contributions. None of the iterative changes proposed have an "unstable
state", they are all bringing value in a series of improvements that fix
the success already made.

Anton

ср, 13 июн. 2018 г. в 22:43, Pedro Larroy <pedro.larroy.lists@gmail.com>:

> The problem is that the process of porting is incremental and requires
> several patches from different collaborators to advance in different areas,
> like build system, infrastructure, code fixes, virtualization.... This gets
> difficult when having multiple scattered PRs open. We lost track of which
> changes where in which PR fixing the ARMv7 port with Anton.
>
> The normal way to operate in these cases in my experience is either use a
> feature branch and collaborate and share patches there, or integrate the
> patches to move towards the goal in the master branch. The latter is not
> always possible. I think going forwards we will try using an integration
> branch in our org:  MXNetEdge/incubator-mxnet which is a public fork. The
> downside is that we should be wary of merging back large patches to master,
> I think often we have problems in large patches that touch too many things.
> Happy to hear different suggestions, as is always good to find better
> branching patterns and ways of working.
>
> Pedro.
>
> On Wed, Jun 13, 2018 at 8:30 PM Thomas DELTEIL <thomas.delteil1@gmail.com>
> wrote:
>
> > Hi Pedro,
> >
> > Is there a problem in working off a branch in your own fork and issue a
> > [WIP] PR ? This is a pattern I have seen a lot and personally I think it
> > works well, since it also gives some visibility if someone is interested
> in
> > looking at the progress of the work. You can add people collaborating
> with
> > you as collaborator to your own fork and that way your commits will be
> run
> > against the CI. Make sure to merge from apache/master and not
> larroy/master
> > if you have conflicts? Not sure why you got these conflicts otherwise.
> >
> > All the best,
> >
> > Thomas
> >
> > 2018-06-12 23:39 GMT-07:00 Pedro Larroy <pedro.larroy.lists@gmail.com>:
> >
> > > Thanks a lot for creating these branches and proposing the idea, for
> the
> > > reasons you listed.
> > >
> > >
> > >  We tried during this week to work with these branches with @lebeg for
> > > Android and Arm support, for the reasons listed below these branches
> are
> > > not useful for us, so you can delete them.
> > >
> > > 1. We don't have permissions to commit to these development branches,
> > > 2. they show merge conflicts that have been solved locally before
> running
> > > CI (?). I'm pretty sure I merged and resolved conflicts locally. 3. It
> > > would also pollute the repository history with continuous merges to and
> > > from these branches. I prefer to have a linear history in master so
> > > changes, regressions and bisecting can be less painful when dealing
> with
> > > issues.
> > >
> > > I think is important to share development and integrate small,
> > incremental
> > > patches towards architecture support, unfortunately these branches
> can't
> > > help us at this stage. We will share our work through a different means
> > and
> > > without polluting the project with additional branches which are not
> > meant
> > > for production or general use.
> > >
> > >
> > >
> > >
> > > On Mon, Jun 11, 2018 at 6:20 AM Marco de Abreu <
> > > marco.g.abreu@googlemail.com>
> > > wrote:
> > >
> > > > The problem with regular reviews here is that we might want to keep
> > > > temporary code or hacks as a temporary solution before we finalize
> it.
> > A
> > > > regular review would have problems with that.
> > > >
> > > > The reason against a fork is the requirement of CI. Since multiple
> > people
> > > > are working on the same branch and we have to file PRs against each
> > > other,
> > > > it would cause problems if CI is only triggered after the fact.
> > > >
> > > > Ideally, the branch will be in a good state and wouldn't need many
> > > chances
> > > > to be mergeable for master.
> > > >
> > > > Naveen Swamy <mnnaveen@gmail.com> schrieb am So., 10. Juni 2018,
> > 10:46:
> > > >
> > > > > I suggest you do a regular review and not a pass-through review for
> > > these
> > > > > branches as well. It would hard to manage a massive review at the
> > end,
> > > if
> > > > > every commit to the branch goes with a proper PR, you could just
> > merge
> > > to
> > > > > the master when its ready.
> > > > >
> > > > > If you want an experimental branch, why not just work it off of a
> > fork?
> > > > >
> > > > >
> > > > > On Thu, Jun 7, 2018 at 5:32 PM, Marco de Abreu <
> > > > > marco.g.abreu@googlemail.com
> > > > > > wrote:
> > > > >
> > > > > > Hello,
> > > > > >
> > > > > > we are currently revisiting our setup for ARM and Android. Since
> > > we're
> > > > > not
> > > > > > in a stable state but need some way to collaborate while having
> > > support
> > > > > > from CI, we proposed the usage of feature branches. Thus, I
have
> > > create
> > > > > the
> > > > > > two branches devel-arm and devel-android into which Anton and
> Pedro
> > > are
> > > > > > going to submit their PRs.
> > > > > >
> > > > > > @Reviewers: It'd be great if you could assist with the reviews.
> > > Please
> > > > > note
> > > > > > that the reviews for these branches can be less harsh, thus
> > temporary
> > > > > > solutions and commented code are totally acceptable. We will
do a
> > > final
> > > > > > review after the acceptance criteria (successful
> cross-compilation
> > > and
> > > > > > manual test execution on a physical device) are fulfilled. This
> is
> > > > > > necessary because we currently have no CI coverage and would
like
> > to
> > > > keep
> > > > > > these changes isolated from master until we found a stable
> > solution.
> > > > > >
> > > > > > Best regards,
> > > > > > Marco
> > > > > >
> > > > >
> > > >
> > >
> >
>

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