Aleksandar, your message is hard to read, because you sent Content-Type:
multipart/alternative, and both the test/html and the test/plain
alternative ride roughshot over the quoted text's line structure.
Quoted patches become unreadable garbage. Please check your e-mail
setup.
Aleksandar Markovic <aleksandar.m.m...@gmail.com> writes:
> 25.09.2019. 17.53, "Philippe Mathieu-Daudé" <phi...@redhat.com> је
> написао/ла:
>>
>> On 9/25/19 2:45 PM, Aleksandar Markovic wrote:
>> > From: Aleksandar Markovic <amarko...@wavecomp.com>
>> >
>> > Mostly fix errors and warnings reported by 'checkpatch.pl -f'.
>> >
>> > Signed-off-by: Aleksandar Markovic <amarko...@wavecomp.com>
>> > ---
>> > target/mips/helper.c | 132
> +++++++++++++++++++++++++++++++--------------------
>> > 1 file changed, 80 insertions(+), 52 deletions(-)
>> >
>> > diff --git a/target/mips/helper.c b/target/mips/helper.c
>> > index a2b6459..3dd1aae 100644
>> > --- a/target/mips/helper.c
>> > +++ b/target/mips/helper.c
>> > @@ -130,8 +133,11 @@ static int is_seg_am_mapped(unsigned int am, bool
> eu, int mmu_idx)
>> > int32_t adetlb_mask;
>> >
>> > switch (mmu_idx) {
>> > - case 3 /* ERL */:
>> > - /* If EU is set, always unmapped */
>> > + case 3:
>> > + /*
>> > + * ERL
>> > + * If EU is set, always unmapped
>> > + */
>>
>> My IDE show the current form nicer when the switch is folded.
>>
>> Are these comment really bothering checkpatch?
>>
>
> While being sintaxically correct, interleaving comments and code in a
> single code line is considered a bad practice by many.
I take that as a "no".
The preferences of "many" (whoever they may be) are a lot less relevant
than the specific project's prevailing style. "git-grep ' case .*/\*'"
shows thousands of hits.
If you want to pursue this change, please put it in a separate patch,
so this one is really about fixing "errors and warnings reported by
'checkpatch.pl -f'", as your commit message promises.
>> > if (eu) {
>> > return 0;
>> > }
[...]
>> Except 2 comments, OK for the rest.
>>
>> Another patch that makes rebasing very painful :(
>>
>
> It would be fantastic if you apply the same reasoning to your patches, that
> spread all over the code base, and happen so frequently, and certainly
> create enormously more rebasing problems for multitude of people than this
> patch or series does.
Please tone down the aggression a notch.
Philippe merely observed a trade-off. We deal with such trade-offs all
the time. When your reviewer challenges one, you consider it (unless
you did that already), then tell him what you concluded.
"Tu quoque" is not a proper reply to such an observation (or to anything
else for that matter). When you have an issue with Philippe's patches,
address it in review of Philippe's patches.
Thank you.
