openbmc.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Ed Tanous <edtanous@google.com>
To: Lei Yu <yulei.sh@bytedance.com>
Cc: openbmc <openbmc@lists.ozlabs.org>,
	Thang Nguyen <thang@amperemail.onmicrosoft.com>
Subject: Re: [External] Re: New test for patches in openbmc/openbmc
Date: Mon, 11 Oct 2021 10:47:24 -0700	[thread overview]
Message-ID: <CAH2-KxDZD0hYdmrbainSHHKhtqp32a=aXmJg1WxyFOtGTSzCEA@mail.gmail.com> (raw)
In-Reply-To: <CAGm54UHq33wD5Fm6D59Mq=XwFrQvXQ0qdbV-CSRwNLuyfj06hg@mail.gmail.com>

On Fri, Oct 8, 2021 at 7:18 PM Lei Yu <yulei.sh@bytedance.com> wrote:
>
> On Sat, Oct 9, 2021 at 1:35 AM Ed Tanous <edtanous@google.com> wrote:
> >
> > On Fri, Oct 8, 2021 at 1:31 AM Lei YU <mine260309@gmail.com> wrote:
> > >
> > > It's noticed that the `repotest` is enabled in CI and we got CI
> > > failure due to node-manager's patch:
> > > https://gerrit.openbmc-project.xyz/c/openbmc/openbmc/+/47673
> > >
> > > I know the right way is to ask Intel to upstream the node-manager and
> > > fix the issues we met.
> > > But in reality it's not easy and it takes time for Intel to upstream a
> > > repo (and it depends on Intel to decide whether or not to upstream)
> >
> > If this is something you need, there's no need to wait for Intel, as
> > that application already has an Apache 2 license.  You are free to
> > upstream it and maintain it yourself if you don't want to wait for
> > intel.
> >
> > >
> > >
> > > @Ed Do we really want to reject such patches?
> >
> > I don't want to reject patches, I want to see them on master in a way
> > that things can be changed as needs evolve.  This patch is a perfect
> > example of something that, had we taken the small amount of time to
> > upstream this small daemon, wouldn't have even been an issue now as
> > sdbusplus needs to make a very minor change.  As-is, we're effectively
>
> Totally agree. We have already asked Intel to upstream the
> node-manager, let's wait for the feedback.'

Let's not wait any longer.  The code is licensed appropriately, is
already open source, and in total, is smaller than a number of single
patchsets I've seen in recent history (it would probably classify as a
medium patchset).  Just open a review to add the code to an existing
repository, or request a new repository along with a design doc.  If
you want this to live in dbus-sensors, I'm fine with that, just make
sure we have maintainers that can test on their systems;  It seems
like an ok fit given it's another entity-manager enabled sensor app,
although I don't have a strong opinion between the two options.


>
> > 2 levels of fork deep (openbmc upstream -> intel-bmc -> openbmc
> > upstream only for bytedance systems, which is the source of the
> > problem, not this patch itself.
>
> True. But as an Intel x86 platform, the repo is needed and in the
> current state, the patch has to be added. Otherwise the g220a build is
> broken.
> Is it OK to ignore the repotest CI failure and just merge the patch in
> meta-bytedance layer?
> (Be noted that it's not trying to make a bad example, but only trying
> to fix the broken build)

  reply	other threads:[~2021-10-11 17:48 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-21 22:35 New test for patches in openbmc/openbmc Ed Tanous
2021-09-22  9:02 ` Alexander Amelkin
2021-09-22 13:15   ` Verdun, Jean-Marie
2021-09-22 23:35   ` Oskar Senft
2021-09-23  2:49     ` John Broadbent
2021-09-23 17:09       ` Benjamin Fair
2021-09-23 23:57       ` Ed Tanous
2021-09-23 23:38   ` Ed Tanous
2021-09-24 10:28     ` Thang Nguyen
2021-09-24 11:06       ` [External] " Lei Yu
2021-09-27 16:33         ` Ed Tanous
2021-09-28  8:36           ` Lei Yu
2021-10-08  8:32             ` Lei YU
2021-10-08 17:35               ` Ed Tanous
2021-10-09  2:18                 ` Lei Yu
2021-10-11 17:47                   ` Ed Tanous [this message]
2021-09-27 16:39       ` Ed Tanous

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAH2-KxDZD0hYdmrbainSHHKhtqp32a=aXmJg1WxyFOtGTSzCEA@mail.gmail.com' \
    --to=edtanous@google.com \
    --cc=openbmc@lists.ozlabs.org \
    --cc=thang@amperemail.onmicrosoft.com \
    --cc=yulei.sh@bytedance.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).