On Thu, Feb 2, 2023 at 4:56 PM Tyler Retzlaff
<roret...@linux.microsoft.com> wrote:
>
> On Thu, Feb 02, 2023 at 10:14:41AM +0100, David Marchand wrote:
> > Hello Tyler,
> >
> > On Fri, Jan 20, 2023 at 11:14 PM Tyler Retzlaff
> > <roret...@linux.microsoft.com> wrote:
> > >
> > > hi folks,
> > >
> > > i think this one can probably be merged?
> > >
> > > Series-acked-by: Morten Brørup <m...@smartsharesystems.com>
> > > Series-acked-by: Bruce Richardson <bruce.richard...@intel.com>
> > > patch 1/2 Acked-by: Ferruh Yigit <ferruh.yi...@amd.com>
> >
> > I like the cleanup of rte_common.h and additional unit tests, but the
> > MSVC bits don't belong here.
> > Please move them in your MSVC enablement series
>
> hm, the way i'm approaching this is to keep specific features together
> similar to if i added new platform functionality for threads i add
> windows or linux in the same series. similarly, in this case i'm adding
> msvc and gcc in the same series.
>
> the msvc enablement series introduces the changes for the build system
> so it's about enabling a compiler not adding functionality to dpdk so
> this change/API really aren't related to the other, but i agree the other
> series is part of converging on the platform, toolchain combination
> being enabled overall.
>
> reconsider? if not i guess i'll just withdraw the actual API for now
> and have to resubmit and review later since i don't think it belongs
> mixed in with the compiler enablement.
>
> > https://patchwork.dpdk.org/project/dpdk/list/?series=26662&state=%2A&archive=both.
>
> if the msvc series were just merged, i think the above discussion would be
> moot no?

Indeed, but until we have support for MSVC, this part of the patch is
putting dead (untested) code in the tree.
It is also confusing to see references to MSVC in the tree while we
have nothing setting this config item, and no documentation explaining
how/when this will be actually usable.


-- 
David Marchand

Reply via email to