Re: [SPDK] [spdk/spdk] Master Pull request for DPDK and Posix abstraction changes. (#152)

Re: [SPDK] [spdk/spdk] Master Pull request for DPDK and Posix abstraction changes. (#152)

[John Meneghini]

[-- Attachment #1: Type: text/plain, Size: 4210 bytes --]

Also, with these changes we are trying to encourage more community participation.  

I think there are many other individuals and companies in the SPDK community who have a stake in these change. As we go through this list of proposed patches (31 in all) I’d like to see some reviews and approvals from others in the community. 

Can we agree upon a criteria for approval with changes? 

I propose a criteria of at least 3 approvals for all commits:

1. from the proposing individual or representative company
2. from the Intel SPDK development team
3. from a third party

Thoughts?

John Meneghini
john.a.meneghini(a)gmail.com

> On May 5, 2017, at 8:22 AM, John Meneghini <john.a.meneghini(a)gmail.com> wrote:
> 
> Hi Ben.
> 
> I agree that commits fbed823 <https://github.com/spdk/spdk/commit/fbed8236facac9833bfd2677b7d76df79cc1c360>, 1bce206 <https://github.com/spdk/spdk/commit/1bce2064db31d44d900f41bab52764dcf65320e8>, edf25e7 <https://github.com/spdk/spdk/commit/edf25e716239ec0051df88207e6f70e474177839> can be dropped.  These were the result of the fact that we are using -Wno-pointer-arith and -Wno-pointer-sign when compiling with clang-3.9.1.   We’ve changed this.  Commit 6ab852f <https://github.com/spdk/spdk/commit/6ab852f9e28d39dc021acbb9a0f0951e6dc933c0> was the result of running lint.  We’ve disabled the process that runs lint on this code so you can drop that SHA as well.
> 
> I will review the new pull requests by the end of today.
> 
> John Meneghini
> john.a.meneghini(a)gmail.com <mailto:john.a.meneghini(a)gmail.com>
> 
> 
> 
>> Begin forwarded message:
>> 
>> From: Ben Walker <notifications(a)github.com <mailto:notifications(a)github.com>>
>> Subject: Re: [spdk/spdk] Master Pull request for DPDK and Posix abstraction changes. (#152)
>> Date: May 3, 2017 at 2:37:14 PM EDT
>> To: spdk/spdk <spdk(a)noreply.github.com <mailto:spdk(a)noreply.github.com>>
>> Cc: John Meneghini <john.a.meneghini(a)gmail.com <mailto:john.a.meneghini(a)gmail.com>>, Author <author(a)noreply.github.com <mailto:author(a)noreply.github.com>>
>> Reply-To: spdk/spdk <reply+00c54bdda6bf8a3c64ce5c17da0be4a955707c78620de88e92cf000000011521e55a92a169ce0d7675a5(a)reply.github.com <mailto:reply+00c54bdda6bf8a3c64ce5c17da0be4a955707c78620de88e92cf000000011521e55a92a169ce0d7675a5(a)reply.github.com>>
>> 
>> I have so far picked off the following:
>> 
>> e2ecccb <https://github.com/spdk/spdk/commit/e2ecccb3423c01ac96a2073ab11c413f2e70f1bc> as pull request #153 <https://github.com/spdk/spdk/pull/153>
>> 32f27bb <https://github.com/spdk/spdk/commit/32f27bb80026e73b73abd7e7f0eb83e9af81e68a> as pull request #154 <https://github.com/spdk/spdk/pull/154>
>> 59072b9 <https://github.com/spdk/spdk/commit/59072b953699628d0447dcf8734473e26807bc26> as pull request #155 <https://github.com/spdk/spdk/pull/155>
>> Note that I made significant modifications to each of these commits as I picked them off. Please review on the relevant PRs.
>> 
>> I also recommend we drop the following commits entirely:
>> fbed823 <https://github.com/spdk/spdk/commit/fbed8236facac9833bfd2677b7d76df79cc1c360>, 1bce206 <https://github.com/spdk/spdk/commit/1bce2064db31d44d900f41bab52764dcf65320e8>, edf25e7 <https://github.com/spdk/spdk/commit/edf25e716239ec0051df88207e6f70e474177839> - These are all adding casts but we're intentionally using the GNU C extension to treat void pointers as uint8_t pointers regarding pointer math. If you are unable to change your compiler settings to allow for this, let me know and we'll address it some other way.
>> 
>> 6ab852f <https://github.com/spdk/spdk/commit/6ab852f9e28d39dc021acbb9a0f0951e6dc933c0> - As far as we can see, this is a false positive from your linter and not an actual problem.
>> 
>> That accounts for 7 out of 31 of the commits so far. I'll continue to cherry pick and update here.
>> 
>> —
>> You are receiving this because you authored the thread.
>> Reply to this email directly, view it on GitHub <https://github.com/spdk/spdk/pull/152#issuecomment-298998284>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AMVL3c0haArPauICl6fTTal_QsyvE6vQks5r2MlagaJpZM4NO4ku>.
>> 
> 


[-- Attachment #2: attachment.html --]
[-- Type: text/html, Size: 12585 bytes --]

Re: [SPDK] [spdk/spdk] Master Pull request for DPDK and Posix abstraction changes. (#152)

[Walker, Benjamin]

[-- Attachment #1: Type: text/plain, Size: 1811 bytes --]

On Fri, 2017-05-05 at 09:24 -0400, John Meneghini wrote:
> Also, with these changes we are trying to encourage more community
> participation.  
> 
> I think there are many other individuals and companies in the SPDK community
> who have a stake in these change. As we go through this list of proposed
> patches (31 in all) I’d like to see some reviews and approvals from others in
> the community. 
> 
> Can we agree upon a criteria for approval with changes? 
> 
> I propose a criteria of at least 3 approvals for all commits:
> 
> 1. from the proposing individual or representative company
> 2. from the Intel SPDK development team
> 3. from a third party
> 
> Thoughts?

Feedback is always welcome from anyone. I encourage anyone with an opinion to
post comments on the pull requests. We'll leave each request open for a few days
minimum to solicit feedback.

The official process is that the SPDK maintainers (currently Daniel, Jim, and
myself) can accept or deny a patch as we see fit. However, in practice that's
going to mean that we've gathered feedback from all of the interested parties
and we're confident that the patch meets everyone's needs. We won't ever just
plow ahead without consensus unless we're not getting any participation.

For this particular series, we're not going to merge anything unless both you
and Ranjit are ok with it (or provide no feedback for an extended period of
time), since you two are the primary stakeholders in this effort. We're also
going to try and poke a few people who have shown interest in this in the past
and try to get some more feedback that way. If anyone else is interested in the
DPDK/POSIX abstractions and starts commenting, we'll make sure their feedback is
all incorporated into the final patches as well.

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3274 bytes --]