Re: [RFC 2/3] xhci: Fix cycle bit calculation during stall handling.

Re: [RFC 2/3] xhci: Fix cycle bit calculation during stall handling.

[Sarah Sharp]

On Thu, Feb 24, 2011 at 02:18:24PM +0300, Sergei Shtylyov wrote:
> Hello.
> 
> On 24-02-2011 5:31, Sarah Sharp wrote:
> 
> >+	if (ep_ring->first_seg == ep_ring->first_seg->next&&
> >+			state->new_deq_ptr < dev->eps[ep_index].stopped_trb)
> >+			state->new_cycle_state ^= 0x1;
> 
>    The last line seems overindented.

Hi Sergei,

checkpatch.pl doesn't complain about it.  It is not a good use of my
time to respin this patch for one stylistic change that is not a
violation of the Kernel community norms.

I really feel strongly that you are wasting the community's time by
commenting on style when it is only a violation of your personal style,
without providing any useful feedback on the code itself.

I feel so strongly that I'm seriously considering setting up a feedback
bot for you.  It would run any patch you reply to through checkpatch.pl,
and send you a small reminder when the patch passes, but you complained
about style.  I think it would allow you to have a sense of how
frustrating it is to receive these unnecessary stylistic comments.

Sarah Sharp

Re: [RFC 2/3] xhci: Fix cycle bit calculation during stall handling.

[Greg KH]

On Thu, Feb 24, 2011 at 07:38:55AM -0800, Sarah Sharp wrote:
> On Thu, Feb 24, 2011 at 02:18:24PM +0300, Sergei Shtylyov wrote:
> > Hello.
> > 
> > On 24-02-2011 5:31, Sarah Sharp wrote:
> > 
> > >+	if (ep_ring->first_seg == ep_ring->first_seg->next&&
> > >+			state->new_deq_ptr < dev->eps[ep_index].stopped_trb)
> > >+			state->new_cycle_state ^= 0x1;
> > 
> >    The last line seems overindented.
> 
> Hi Sergei,
> 
> checkpatch.pl doesn't complain about it.  It is not a good use of my
> time to respin this patch for one stylistic change that is not a
> violation of the Kernel community norms.
> 
> I really feel strongly that you are wasting the community's time by
> commenting on style when it is only a violation of your personal style,
> without providing any useful feedback on the code itself.
> 
> I feel so strongly that I'm seriously considering setting up a feedback
> bot for you.  It would run any patch you reply to through checkpatch.pl,
> and send you a small reminder when the patch passes, but you complained
> about style.  I think it would allow you to have a sense of how
> frustrating it is to receive these unnecessary stylistic comments.

I totally agree, and almost always just filter Sergei's comments away as
the large majority of them are pointless and distract from the technical
content being discussed.

Sergei, you have been warned of this before, please change in the
future.

thanks,

greg k-h

Re: [RFC 2/3] xhci: Fix cycle bit calculation during stall handling.

[Dmitry Torokhov]

On Thu, Feb 24, 2011 at 09:04:10AM -0800, Greg KH wrote:
> On Thu, Feb 24, 2011 at 07:38:55AM -0800, Sarah Sharp wrote:
> > On Thu, Feb 24, 2011 at 02:18:24PM +0300, Sergei Shtylyov wrote:
> > > Hello.
> > > 
> > > On 24-02-2011 5:31, Sarah Sharp wrote:
> > > 
> > > >+	if (ep_ring->first_seg == ep_ring->first_seg->next&&
> > > >+			state->new_deq_ptr < dev->eps[ep_index].stopped_trb)
> > > >+			state->new_cycle_state ^= 0x1;
> > > 
> > >    The last line seems overindented.
> > 
> > Hi Sergei,
> > 
> > checkpatch.pl doesn't complain about it.  It is not a good use of my
> > time to respin this patch for one stylistic change that is not a
> > violation of the Kernel community norms.
> > 
> > I really feel strongly that you are wasting the community's time by
> > commenting on style when it is only a violation of your personal style,
> > without providing any useful feedback on the code itself.
> > 
> > I feel so strongly that I'm seriously considering setting up a feedback
> > bot for you.  It would run any patch you reply to through checkpatch.pl,
> > and send you a small reminder when the patch passes, but you complained
> > about style.  I think it would allow you to have a sense of how
> > frustrating it is to receive these unnecessary stylistic comments.
> 
> I totally agree, and almost always just filter Sergei's comments away as
> the large majority of them are pointless and distract from the technical
> content being discussed.
> 

I am sorry, but in this case the 'if' body does run together with the
conditional. No need to repost the patch for this, of course, but might
be worth fixing locally before pushing into a public branch.

Thanks.

-- 
Dmitry

Re: [RFC 2/3] xhci: Fix cycle bit calculation during stall handling.

[Sergei Shtylyov]

Hello.

Sarah Sharp wrote:

>>> +	if (ep_ring->first_seg == ep_ring->first_seg->next&&
>>> +			state->new_deq_ptr < dev->eps[ep_index].stopped_trb)
>>> +			state->new_cycle_state ^= 0x1;

>>    The last line seems overindented.

> Hi Sergei,

> checkpatch.pl doesn't complain about it.  It is not a good use of my
> time to respin this patch for one stylistic change that is not a
> violation of the Kernel community norms.

> I really feel strongly that you are wasting the community's time by
> commenting on style when it is only a violation of your personal style,
> without providing any useful feedback on the code itself.

    Not only of my personal style, as Dmitry's comment has shown. Like him, I 
had some trouble seeing where the *if* expression ends and the branch starts, 
due to both lines starting on the same level of indentation...

> I feel so strongly that I'm seriously considering setting up a feedback
> bot for you.  It would run any patch you reply to through checkpatch.pl,
> and send you a small reminder when the patch passes, but you complained
> about style.  I think it would allow you to have a sense of how
> frustrating it is to receive these unnecessary stylistic comments.

    Well, I'll try to save you that trouble and avoid commenting on your patches 
in the future.

> Sarah Sharp

WBR, Sergei