Could someone help to review bmcweb code?

Could someone help to review bmcweb code?

[Carol Wang]

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

Hi,

I wanna know who can help to review bmcweb code? I'm not sure if the list
of maintainers I added
is changed or not. This is the link of my coding about setting PowerCap.
---->
https://gerrit.openbmc-project.xyz/c/openbmc/bmcweb/+/26223

Thanks in advance!

[-- Attachment #2: Type: text/html, Size: 457 bytes --]

Re: Could someone help to review bmcweb code?

[James Feist]

On 10/28/19 5:51 AM, Carol Wang wrote:
> Hi,
> 
> I wanna know who can help to review bmcweb code? I'm not sure if the 
> list of maintainers I added
> is changed or not. This is the link of my coding about setting PowerCap. 
> ---->
> https://gerrit.openbmc-project.xyz/c/openbmc/bmcweb/+/26223

It looks like this patch has been -1ed for over a week, generally the 
maintainers will not look at a patch that has been -1ed. I would work 
with the community to come to resolution and get some +1s.

Thanks

-James

> 
> Thanks in advance!

Re: Could someone help to review bmcweb code?

[Brad Bishop]

> On Oct 28, 2019, at 1:08 PM, James Feist <james.feist@linux.intel.com> wrote:
> 
> On 10/28/19 5:51 AM, Carol Wang wrote:
>> Hi,
>> I wanna know who can help to review bmcweb code? I'm not sure if the list of maintainers I added
>> is changed or not. This is the link of my coding about setting PowerCap. ---->
>> https://gerrit.openbmc-project.xyz/c/openbmc/bmcweb/+/26223
> 
> It looks like this patch has been -1ed for over a week,

Actually it got the -1 just last Wednesday.  Prior to that it had a +1 without any -1s for 6 days.

> generally the maintainers will not look at a patch that has been -1ed.

I don’t agree with this and I’d suggest you don’t make this your general policy.  If you do this, then you are going to miss reviewers giving people bad advice, which the reviewers will then implement at great waste of time.  I see kernel maintainers correcting bad advice from reviewers all the time.

> I would work with the community to come to resolution and get some +1s.

Carol responded to the feedback (with questions, that remain unanswered) the same day it was given, 5 days ago.  I’d say "working with the community" is exactly what Carol is trying to do here.

Re: Could someone help to review bmcweb code?

[James Feist]

On 10/28/19 10:47 AM, Brad Bishop wrote:
> 
>> On Oct 28, 2019, at 1:08 PM, James Feist <james.feist@linux.intel.com> wrote:
>>
>> On 10/28/19 5:51 AM, Carol Wang wrote:
>>> Hi,
>>> I wanna know who can help to review bmcweb code? I'm not sure if the list of maintainers I added
>>> is changed or not. This is the link of my coding about setting PowerCap. ---->
>>> https://gerrit.openbmc-project.xyz/c/openbmc/bmcweb/+/26223
>>
>> It looks like this patch has been -1ed for over a week,
> 
> Actually it got the -1 just last Wednesday.  Prior to that it had a +1 without any -1s for 6 days.

It was WIP on the 17th, and +1ed on the 23rd? The same day it was -1ed. 
But we're splitting hairs. Regardless it's been -1ed for quite a few 
days, and it would make the process better if at least the things that 
both Appu and Carol agreed with were fixed. However a direct email to 
Appu would be good to get him to reply to comments.

> 
>> generally the maintainers will not look at a patch that has been -1ed.
> 
> I don’t agree with this and I’d suggest you don’t make this your general policy.  If you do this, then you are going to miss reviewers giving people bad advice, which the reviewers will then implement at great waste of time.  I see kernel maintainers correcting bad advice from reviewers all the time.

Sure, but anyone should be able to say that the advice is bad. And if 
the back and forth isn't working, then I agree that it can be escalated. 
I'm not saying it is a general rule, but most repos won't look at code 
if it is -1ed, and some wont look unless it is +1ed. I know I've been 
told many times that my code can't be merged unless someone on my team 
+1s it.


> 
>> I would work with the community to come to resolution and get some +1s.
> 
> Carol responded to the feedback (with questions, that remain unanswered) the same day it was given, 5 days ago.  I’d say "working with the community" is exactly what Carol is trying to do here.
>

Re: Could someone help to review bmcweb code?

[Brad Bishop]

> On Oct 28, 2019, at 2:13 PM, James Feist <james.feist@linux.intel.com> wrote:
> 
> On 10/28/19 10:47 AM, Brad Bishop wrote:
>>> On Oct 28, 2019, at 1:08 PM, James Feist <james.feist@linux.intel.com> wrote:
>>> 
>>> On 10/28/19 5:51 AM, Carol Wang wrote:
>>>> Hi,
>>>> I wanna know who can help to review bmcweb code? I'm not sure if the list of maintainers I added
>>>> is changed or not. This is the link of my coding about setting PowerCap. ---->
>>>> https://gerrit.openbmc-project.xyz/c/openbmc/bmcweb/+/26223
>>> 
>>> It looks like this patch has been -1ed for over a week,
>> Actually it got the -1 just last Wednesday.  Prior to that it had a +1 without any -1s for 6 days.
> 
> It was WIP on the 17th, and +1ed on the 23rd? The same day it was -1ed. But we're splitting hairs.

I see that now, my bad.

> Regardless it's been -1ed for quite a few days,


> and it would make the process better if at least the things that both Appu and Carol agreed with were fixed.

I’ve not run across this expectation before.  Good to know.

> However a direct email to Appu would be good to get him to reply to comments.

I don’t do direct email and I ask my team not to as well, so I’ll do it here - Appu could you reply to Carol’s questions?

> 
>>> generally the maintainers will not look at a patch that has been -1ed.
>> I don’t agree with this and I’d suggest you don’t make this your general policy.  If you do this, then you are going to miss reviewers giving people bad advice, which the reviewers will then implement at great waste of time.  I see kernel maintainers correcting bad advice from reviewers all the time.
> 
> Sure, but anyone should be able to say that the advice is bad. And if the back and forth isn't working, then I agree that it can be escalated. I'm not saying it is a general rule, but most repos won't look at code if it is -1ed, and some wont look unless it is +1ed. I know I've been told many times that my code can't be merged unless someone on my team +1s it.

Ok - yes, I can’t argue with any of your logic here.

-brad

Re: Could someone help to review bmcweb code?

[James Feist]

On 10/28/19 11:42 AM, Brad Bishop wrote:
>> However a direct email to Appu would be good to get him to reply to comments.
> 
> I don’t do direct email and I ask my team not to as well, so I’ll do it here - Appu could you reply to Carol’s questions?

I agree.. I mean direct CC'ing the list :)

Re: Could someone help to review bmcweb code?

[Puli, Apparao]

Yeah, on it.

On 10/29/2019 12:18 AM, James Feist wrote:
> On 10/28/19 11:42 AM, Brad Bishop wrote:
>>> However a direct email to Appu would be good to get him to reply to 
>>> comments.
>>
>> I don’t do direct email and I ask my team not to as well, so I’ll do 
>> it here - Appu could you reply to Carol’s questions?
>
> I agree.. I mean direct CC'ing the list :)
>
>
>

Re: Could someone help to review bmcweb code?

[Brad Bishop]

> On Oct 28, 2019, at 2:49 PM, Puli, Apparao <apparao.puli@linux.intel.com> wrote:
> 
> Yeah, on it.

thanks AppaRao!

-brad

Re: Could someone help to review bmcweb code?

[Carol Wang]

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

Thank you all for your quick response!
I was thinking if this commit was missed. Now I'm sure I'll be more
familiar with the reviewing process.

On Tue, Oct 29, 2019 at 2:55 AM Brad Bishop <bradleyb@fuzziesquirrel.com>
wrote:

> > On Oct 28, 2019, at 2:49 PM, Puli, Apparao <apparao.puli@linux.intel.com>
> wrote:
> >
> > Yeah, on it.
>
> thanks AppaRao!
>
> -brad

[-- Attachment #2: Type: text/html, Size: 783 bytes --]

Re: Could someone help to review bmcweb code?

[Puli, Apparao]

On 10/28/2019 11:17 PM, Brad Bishop wrote:
>> On Oct 28, 2019, at 1:08 PM, James Feist <james.feist@linux.intel.com> wrote:
>>
>> On 10/28/19 5:51 AM, Carol Wang wrote:
>>> Hi,
>>> I wanna know who can help to review bmcweb code? I'm not sure if the list of maintainers I added
>>> is changed or not. This is the link of my coding about setting PowerCap. ---->
>>> https://gerrit.openbmc-project.xyz/c/openbmc/bmcweb/+/26223
>> It looks like this patch has been -1ed for over a week,
> Actually it got the -1 just last Wednesday.  Prior to that it had a +1 without any -1s for 6 days.
>
>> generally the maintainers will not look at a patch that has been -1ed.
> I don’t agree with this and I’d suggest you don’t make this your general policy.  If you do this, then you are going to miss reviewers giving people bad advice, which the reviewers will then implement at great waste of time.  I see kernel maintainers correcting bad advice from reviewers all the time.
>
>> I would work with the community to come to resolution and get some +1s.
> Carol responded to the feedback (with questions, that remain unanswered) the same day it was given, 5 days ago.  I’d say "working with the community" is exactly what Carol is trying to do here.

I am waiting for new patch-set from Carol to the one's which was 
mentioned that those are going to be modified and loaded new patch set.

Re: Could someone help to review bmcweb code?

[Brad Bishop]

> On Oct 28, 2019, at 2:40 PM, Puli, Apparao <apparao.puli@linux.intel.com> wrote:
> 
> I am waiting for new patch-set from Carol to the one's which was mentioned that those are going to be modified and loaded new patch set.

Before even answering his questions?

Is this really the community norm we want to establish?  Questions are simply ignored without explanation?