Re: [PATCH v2] certs/extract-cert: Fix checkpatch issues

Re: [PATCH v2] certs/extract-cert: Fix checkpatch issues

[Bagas Sanjaya]

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

On Fri, Jun 02, 2023 at 10:59:02AM +0200, Franziska Naepelt wrote:
> The following issues are fixed:
> - WARNING: Missing or malformed SPDX-License-Identifier tag
> - ERROR: trailing statements should be on next line
> - WARNING: braces {} are not necessary for single statement blocks
> - ERROR: space required before the open parenthesis '('
> - ERROR: code indent should use tabs where possible
> - WARNING: please, no spaces at the start of a line
> - WARNING: Missing a blank line after declarations

Again, write the patch description in imperative mood (e.g. "Do foo").

> +// SPDX-License-Identifier: LGPL-2.1
>  /* Extract X.509 certificate in DER form from PKCS#11 or PEM.
>   *
>   * Copyright © 2014-2015 Red Hat, Inc. All Rights Reserved.

Nope.

The license boilerplate says LGPL 2.1 or any later version, so the
corresponding SPDX tag should have been:

```
// SPDX-License-Identifier: LGPL-2.1-or-later
```

And please also delete the boilerplate and separate this SPDX conversion
into its own patch.

Thanks.

-- 
An old man doll... just what I always wanted! - Clara

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2] certs/extract-cert: Fix checkpatch issues

[Dan Carpenter]

On Tue, Jun 06, 2023 at 07:28:52PM +0700, Bagas Sanjaya wrote:
> On Fri, Jun 02, 2023 at 10:59:02AM +0200, Franziska Naepelt wrote:
> > The following issues are fixed:
> > - WARNING: Missing or malformed SPDX-License-Identifier tag
> > - ERROR: trailing statements should be on next line
> > - WARNING: braces {} are not necessary for single statement blocks
> > - ERROR: space required before the open parenthesis '('
> > - ERROR: code indent should use tabs where possible
> > - WARNING: please, no spaces at the start of a line
> > - WARNING: Missing a blank line after declarations
> 
> Again, write the patch description in imperative mood (e.g. "Do foo").
> 

Why do you care about imperative tense?  Imperative tense doesn't
matter.  What matters is that you can understand the issue and how it
looks like to the user.  I was working with a group of foreign students
and it was painful to see the contortions that they went through to make
a commit message imperative.  It's like saying "Bake a cake", "Ok, now
bake it while juggling."  The cake ends up worse.  And the commit
message end up worse when we force nonsense rules like this.

That said the rest of your comments are correct.

Franziska, I kind of feel like you should start in drivers/staging/
because you're sending beginner patches to kind of core parts of the
kernel where people are busy.  Most likely your going to have to send a
bunch of iterations of the patch.  In drivers/staging/ we don't mind
reviewing newbie patches.

regards,
dan carpenter

Re: [PATCH v2] certs/extract-cert: Fix checkpatch issues

[Jarkko Sakkinen]

On Tue Jun 6, 2023 at 4:38 PM EEST, Dan Carpenter wrote:
> On Tue, Jun 06, 2023 at 07:28:52PM +0700, Bagas Sanjaya wrote:
> > On Fri, Jun 02, 2023 at 10:59:02AM +0200, Franziska Naepelt wrote:
> > > The following issues are fixed:
> > > - WARNING: Missing or malformed SPDX-License-Identifier tag
> > > - ERROR: trailing statements should be on next line
> > > - WARNING: braces {} are not necessary for single statement blocks
> > > - ERROR: space required before the open parenthesis '('
> > > - ERROR: code indent should use tabs where possible
> > > - WARNING: please, no spaces at the start of a line
> > > - WARNING: Missing a blank line after declarations
> > 
> > Again, write the patch description in imperative mood (e.g. "Do foo").
> > 
>
> Why do you care about imperative tense?  Imperative tense doesn't
> matter.  What matters is that you can understand the issue and how it
> looks like to the user.  I was working with a group of foreign students
> and it was painful to see the contortions that they went through to make
> a commit message imperative.  It's like saying "Bake a cake", "Ok, now
> bake it while juggling."  The cake ends up worse.  And the commit
> message end up worse when we force nonsense rules like this.

How about a simple and stupid reason?

Usually I write commit message without caring about this. Then I rewrite
the commit message and 9/10 it gets shorter. Based on empirical
experience, imperative form has minimum amount of extra words.

I came up with this convention first when submitting patches to x86, and
over time it has started to make sense to me.

Especially for multi patch sets too verbose language tends to be super
tiring for non-native English speaker. One should optimize the language
in those: say *exactly* what is needed, and not more. If this not the
case, I tend to move these patch sets very last in my queue.

It's not a "punishment". It's more like that I really have to take the
time to read the prose...

BR, Jarkko

Re: [PATCH v2] certs/extract-cert: Fix checkpatch issues

[Dan Carpenter]

On Tue, Jun 06, 2023 at 05:51:09PM +0300, Jarkko Sakkinen wrote:
> On Tue Jun 6, 2023 at 4:38 PM EEST, Dan Carpenter wrote:
> > On Tue, Jun 06, 2023 at 07:28:52PM +0700, Bagas Sanjaya wrote:
> > > On Fri, Jun 02, 2023 at 10:59:02AM +0200, Franziska Naepelt wrote:
> > > > The following issues are fixed:
> > > > - WARNING: Missing or malformed SPDX-License-Identifier tag
> > > > - ERROR: trailing statements should be on next line
> > > > - WARNING: braces {} are not necessary for single statement blocks
> > > > - ERROR: space required before the open parenthesis '('
> > > > - ERROR: code indent should use tabs where possible
> > > > - WARNING: please, no spaces at the start of a line
> > > > - WARNING: Missing a blank line after declarations
> > > 
> > > Again, write the patch description in imperative mood (e.g. "Do foo").
> > > 
> >
> > Why do you care about imperative tense?  Imperative tense doesn't
> > matter.  What matters is that you can understand the issue and how it
> > looks like to the user.  I was working with a group of foreign students
> > and it was painful to see the contortions that they went through to make
> > a commit message imperative.  It's like saying "Bake a cake", "Ok, now
> > bake it while juggling."  The cake ends up worse.  And the commit
> > message end up worse when we force nonsense rules like this.
> 
> How about a simple and stupid reason?
> 
> Usually I write commit message without caring about this. Then I rewrite
> the commit message and 9/10 it gets shorter. Based on empirical
> experience, imperative form has minimum amount of extra words.
> 

I'm looking through the git log to see if it's true the imperative tense
commit message are shorter and better and neither one of those things is
obvious to me.

This patch had an imperative subject already so it was already kind of
imperative.  Does every sentence have to be imperative or can you just
add a "Fix it." to the end?

I don't want to belittle the challenges you face around the English
language but I think students were less fluent than you are.  So maybe
imperative tense works for you but it definitely made their commit
messages far worse.

regards,
dan carpenter

Re: [PATCH v2] certs/extract-cert: Fix checkpatch issues

[Jarkko Sakkinen]

On Tue Jun 6, 2023 at 6:25 PM EEST, Dan Carpenter wrote:
> On Tue, Jun 06, 2023 at 05:51:09PM +0300, Jarkko Sakkinen wrote:
> > On Tue Jun 6, 2023 at 4:38 PM EEST, Dan Carpenter wrote:
> > > On Tue, Jun 06, 2023 at 07:28:52PM +0700, Bagas Sanjaya wrote:
> > > > On Fri, Jun 02, 2023 at 10:59:02AM +0200, Franziska Naepelt wrote:
> > > > > The following issues are fixed:
> > > > > - WARNING: Missing or malformed SPDX-License-Identifier tag
> > > > > - ERROR: trailing statements should be on next line
> > > > > - WARNING: braces {} are not necessary for single statement blocks
> > > > > - ERROR: space required before the open parenthesis '('
> > > > > - ERROR: code indent should use tabs where possible
> > > > > - WARNING: please, no spaces at the start of a line
> > > > > - WARNING: Missing a blank line after declarations
> > > > 
> > > > Again, write the patch description in imperative mood (e.g. "Do foo").
> > > > 
> > >
> > > Why do you care about imperative tense?  Imperative tense doesn't
> > > matter.  What matters is that you can understand the issue and how it
> > > looks like to the user.  I was working with a group of foreign students
> > > and it was painful to see the contortions that they went through to make
> > > a commit message imperative.  It's like saying "Bake a cake", "Ok, now
> > > bake it while juggling."  The cake ends up worse.  And the commit
> > > message end up worse when we force nonsense rules like this.
> > 
> > How about a simple and stupid reason?
> > 
> > Usually I write commit message without caring about this. Then I rewrite
> > the commit message and 9/10 it gets shorter. Based on empirical
> > experience, imperative form has minimum amount of extra words.
> > 
>
> I'm looking through the git log to see if it's true the imperative tense
> commit message are shorter and better and neither one of those things is
> obvious to me.
>
> This patch had an imperative subject already so it was already kind of
> imperative.  Does every sentence have to be imperative or can you just
> add a "Fix it." to the end?
>
> I don't want to belittle the challenges you face around the English
> language but I think students were less fluent than you are.  So maybe
> imperative tense works for you but it definitely made their commit
> messages far worse.

Yeah, I was not trying to oppose, just reasoning why I like it more.

For a single patch, this does not really matter anyway :-)

BR, Jarkko

Re: [PATCH v2] certs/extract-cert: Fix checkpatch issues

[Franziska Näpelt]

Am Di., 6. Juni 2023 um 18:03 Uhr schrieb Jarkko Sakkinen <jarkko@kernel.org>:
> On Tue Jun 6, 2023 at 6:25 PM EEST, Dan Carpenter wrote:
> > On Tue, Jun 06, 2023 at 05:51:09PM +0300, Jarkko Sakkinen wrote:
> > > On Tue Jun 6, 2023 at 4:38 PM EEST, Dan Carpenter wrote:
> > > > On Tue, Jun 06, 2023 at 07:28:52PM +0700, Bagas Sanjaya wrote:
> > > > > On Fri, Jun 02, 2023 at 10:59:02AM +0200, Franziska Naepelt wrote:
> > > > > > The following issues are fixed:
> > > > > > - WARNING: Missing or malformed SPDX-License-Identifier tag
> > > > > > - ERROR: trailing statements should be on next line
> > > > > > - WARNING: braces {} are not necessary for single statement blocks
> > > > > > - ERROR: space required before the open parenthesis '('
> > > > > > - ERROR: code indent should use tabs where possible
> > > > > > - WARNING: please, no spaces at the start of a line
> > > > > > - WARNING: Missing a blank line after declarations
> > > > >
> > > > > Again, write the patch description in imperative mood (e.g. "Do foo").
> > > > >
> > > >
> > > > Why do you care about imperative tense?  Imperative tense doesn't
> > > > matter.  What matters is that you can understand the issue and how it
> > > > looks like to the user.  I was working with a group of foreign students
> > > > and it was painful to see the contortions that they went through to make
> > > > a commit message imperative.  It's like saying "Bake a cake", "Ok, now
> > > > bake it while juggling."  The cake ends up worse.  And the commit
> > > > message end up worse when we force nonsense rules like this.
> > >
> > > How about a simple and stupid reason?
> > >
> > > Usually I write commit message without caring about this. Then I rewrite
> > > the commit message and 9/10 it gets shorter. Based on empirical
> > > experience, imperative form has minimum amount of extra words.
> > >
> >
> > I'm looking through the git log to see if it's true the imperative tense
> > commit message are shorter and better and neither one of those things is
> > obvious to me.
> >
> > This patch had an imperative subject already so it was already kind of
> > imperative.  Does every sentence have to be imperative or can you just
> > add a "Fix it." to the end?
> >
> > I don't want to belittle the challenges you face around the English
> > language but I think students were less fluent than you are.  So maybe
> > imperative tense works for you but it definitely made their commit
> > messages far worse.
>
> Yeah, I was not trying to oppose, just reasoning why I like it more.
>
> For a single patch, this does not really matter anyway :-)
>
> BR, Jarkko

I'm a bit puzzled now since there are different opinions on my patch.
I'm struggling to draw a conclusion whether to split the patch into smaller
single line patches or not.

I'd propose to split it into two patches:
* One for SPDX license tag fix
* One for spacing, tab, blank line, unnecessary braces etc.
And fix the remarks related to SPDX license tag and the use of imperative.

If you agree I'm happy to provide two new patches.

Anyway, as per Dan's proposal I'll continue to work in drivers/staging.

Thanks,
Franziska

Re: [PATCH v2] certs/extract-cert: Fix checkpatch issues

[Dan Carpenter]

On Tue, Jun 06, 2023 at 07:59:02PM +0200, Franziska Näpelt wrote:
> Am Di., 6. Juni 2023 um 18:03 Uhr schrieb Jarkko Sakkinen <jarkko@kernel.org>:
> > On Tue Jun 6, 2023 at 6:25 PM EEST, Dan Carpenter wrote:
> > > On Tue, Jun 06, 2023 at 05:51:09PM +0300, Jarkko Sakkinen wrote:
> > > > On Tue Jun 6, 2023 at 4:38 PM EEST, Dan Carpenter wrote:
> > > > > On Tue, Jun 06, 2023 at 07:28:52PM +0700, Bagas Sanjaya wrote:
> > > > > > On Fri, Jun 02, 2023 at 10:59:02AM +0200, Franziska Naepelt wrote:
> > > > > > > The following issues are fixed:
> > > > > > > - WARNING: Missing or malformed SPDX-License-Identifier tag
> > > > > > > - ERROR: trailing statements should be on next line
> > > > > > > - WARNING: braces {} are not necessary for single statement blocks
> > > > > > > - ERROR: space required before the open parenthesis '('
> > > > > > > - ERROR: code indent should use tabs where possible
> > > > > > > - WARNING: please, no spaces at the start of a line
> > > > > > > - WARNING: Missing a blank line after declarations
> > > > > >

[ snip ]

> 
> I'm a bit puzzled now since there are different opinions on my patch.
> I'm struggling to draw a conclusion whether to split the patch into smaller
> single line patches or not.
> 
> I'd propose to split it into two patches:
> * One for SPDX license tag fix
> * One for spacing, tab, blank line, unnecessary braces etc.

You should definitely pull the SPDX change into its own patch because
it's sightly controversial and important.

In drivers/staging/ we would say pull each type of checkpatch warning
into its own patch so it would be something like 6 patches.  But I don't
know how it's done in this subsystem.  I feel like Greg maybe goes
overboard on splitting patches up, but the advantage of Greg's system is
that it's easy to explain the rules to newbies.  There is a lot about
staging/ which designed around newbies.

If I'm totally honest, in a lot of subsystems the policy is just leave
it alone.  Don't bother cleaning up checkpatch stuff because it just
creates more work and makes the git log noisier.

regards,
dan carpenter

Re: [PATCH v2] certs/extract-cert: Fix checkpatch issues

[Ben Boeckel]

On Tue, Jun 06, 2023 at 18:25:24 +0300, Dan Carpenter wrote:
> I'm looking through the git log to see if it's true the imperative tense
> commit message are shorter and better and neither one of those things is
> obvious to me.
> 
> This patch had an imperative subject already so it was already kind of
> imperative.  Does every sentence have to be imperative or can you just
> add a "Fix it." to the end?

I don't know about the length argument, but it feels like it reads
better when skimming summaries with the imperative mood. The way I think
about it is that the subject should complete the phrase:

    When applied, this patch will…

The body then gives more context and description as necessary. I don't
really worry so much about the mood/tense/whatever in the body except
that I try to use the present tense for anything the patch is doing and
past for any historical context. I understand that kernel maintainers
may care a lot more about it though.

Basically, a patch, on its own, does nothing (just like a recipe). It is
only when it is applied that anything actually happens. I read it as
"`git apply`, please $summary".

--Ben

Re: [PATCH v2] certs/extract-cert: Fix checkpatch issues

[Jarkko Sakkinen]

On Wed Jun 7, 2023 at 12:43 AM EEST, Ben Boeckel wrote:
> On Tue, Jun 06, 2023 at 18:25:24 +0300, Dan Carpenter wrote:
> > I'm looking through the git log to see if it's true the imperative tense
> > commit message are shorter and better and neither one of those things is
> > obvious to me.
> > 
> > This patch had an imperative subject already so it was already kind of
> > imperative.  Does every sentence have to be imperative or can you just
> > add a "Fix it." to the end?
>
> I don't know about the length argument, but it feels like it reads
> better when skimming summaries with the imperative mood. The way I think
> about it is that the subject should complete the phrase:
>
>     When applied, this patch will…
>
> The body then gives more context and description as necessary. I don't
> really worry so much about the mood/tense/whatever in the body except
> that I try to use the present tense for anything the patch is doing and
> past for any historical context. I understand that kernel maintainers
> may care a lot more about it though.
>
> Basically, a patch, on its own, does nothing (just like a recipe). It is
> only when it is applied that anything actually happens. I read it as
> "`git apply`, please $summary".
>
> --Ben

+1

BR, Jarkko

Re: [PATCH v2] certs/extract-cert: Fix checkpatch issues

[Dan Carpenter]

On Tue, Jun 06, 2023 at 05:51:09PM +0300, Jarkko Sakkinen wrote:
> It's not a "punishment". It's more like that I really have to take the
> time to read the prose...

The thing about imperative tense is that it was used as a punishment on
me once five years ago.  I wrote a quite bad commit message and a senior
maintainer told me to re-write it properly and I realized that it was
true.  My commit message was bad.  So I wrote a proper commit message.
And then he yelled at me, "Can't you follow simple directions and write
it in imperative tense like the documentation says?  Are you a
shithead?"

So then I swore I would never talk to him again or to anyone who
enforced the imperative tense rule.  That has only happened once in the
intervening years.  I told the maintainer, "Fine.  Re-write the commit
message however you like and give me Reported-by credit."  This was a
cheeky response and it made the maintainer enraged.  I guess he thought
that my boss would force me to fix the bug or something?  I felt bad for
the Intel developer who had to fix my bug instead because I knew that
the maintainer was going to be super angry if he gave me reported-by
credit so I had put him in a bind.  I almost re-wrote the commit message
so that he wouldn't have to deal with that.  Maybe this is how mothers
feel when they try to take abuse from an angry husband instead of
letting their kids suffer.  But I am a bad mother and I left.

My boss would never have forced me to deal with that.  When he left for
a different company he said, "Dan, I'm transitioning and XXX is taking
over me and I have told him all your weirdness so he is prepared."  And
it was a huge comfort to me because I know what my weakness are.

You people on this thread all seem super nice.  And you're right that we
should always try to be improve every aspect of our craft.

When Jarkko talked about people who write too long commit messages, I
thought about one developer in particular who writes too long commit
messages.  He writes in imperative tense.  He takes everything so
seriously and he's never seen a rule without following it.  His patches
are always right.  People have told him that his commit messages are bad
and too long and those people are right.  But they need to shut up.  The
good things that he does and the bad things that he does are all part of
the same package.  He can't change and I don't want him to feel anything
but welcome.

It's hard to be a good kernel developer without being at least slightly
obsessive.  Both developers and maintainers are that way.  And I deal
with a lot of people and accomodating maintainers you disagree with is
part of the job.

So long as everyone is kind to each other.  That's the main thing.

regards,
dan carpenter

Re: [PATCH v2] certs/extract-cert: Fix checkpatch issues

[Jarkko Sakkinen]

On Fri Jun 9, 2023 at 5:01 PM EEST, Dan Carpenter wrote:
> On Tue, Jun 06, 2023 at 05:51:09PM +0300, Jarkko Sakkinen wrote:
> > It's not a "punishment". It's more like that I really have to take the
> > time to read the prose...
>
> The thing about imperative tense is that it was used as a punishment on
> me once five years ago.  I wrote a quite bad commit message and a senior
> maintainer told me to re-write it properly and I realized that it was
> true.  My commit message was bad.  So I wrote a proper commit message.
> And then he yelled at me, "Can't you follow simple directions and write
> it in imperative tense like the documentation says?  Are you a
> shithead?"

Wow :-O I'm totally against name calling or any sort of shittiness like
that, and all for co-operation. Just told my personal thoughts on the
matter. I'm sorry that this happened to you.

>
> So then I swore I would never talk to him again or to anyone who
> enforced the imperative tense rule.  That has only happened once in the
> intervening years.  I told the maintainer, "Fine.  Re-write the commit
> message however you like and give me Reported-by credit."  This was a
> cheeky response and it made the maintainer enraged.  I guess he thought
> that my boss would force me to fix the bug or something?  I felt bad for
> the Intel developer who had to fix my bug instead because I knew that
> the maintainer was going to be super angry if he gave me reported-by
> credit so I had put him in a bind.  I almost re-wrote the commit message
> so that he wouldn't have to deal with that.  Maybe this is how mothers
> feel when they try to take abuse from an angry husband instead of
> letting their kids suffer.  But I am a bad mother and I left.
>
> My boss would never have forced me to deal with that.  When he left for
> a different company he said, "Dan, I'm transitioning and XXX is taking
> over me and I have told him all your weirdness so he is prepared."  And
> it was a huge comfort to me because I know what my weakness are.
>
> You people on this thread all seem super nice.  And you're right that we
> should always try to be improve every aspect of our craft.
>
> When Jarkko talked about people who write too long commit messages, I
> thought about one developer in particular who writes too long commit
> messages.  He writes in imperative tense.  He takes everything so
> seriously and he's never seen a rule without following it.  His patches
> are always right.  People have told him that his commit messages are bad
> and too long and those people are right.  But they need to shut up.  The
> good things that he does and the bad things that he does are all part of
> the same package.  He can't change and I don't want him to feel anything
> but welcome.
>
> It's hard to be a good kernel developer without being at least slightly
> obsessive.  Both developers and maintainers are that way.  And I deal
> with a lot of people and accomodating maintainers you disagree with is
> part of the job.
>
> So long as everyone is kind to each other.  That's the main thing.

I 110% agree with this. I even bookmarked this response :-)

> regards,
> dan carpenter

BR, Jarkko

Re: [PATCH v2] certs/extract-cert: Fix checkpatch issues

[Jarkko Sakkinen]

On Fri Jun 9, 2023 at 6:37 PM EEST, Jarkko Sakkinen wrote:
> On Fri Jun 9, 2023 at 5:01 PM EEST, Dan Carpenter wrote:
> > On Tue, Jun 06, 2023 at 05:51:09PM +0300, Jarkko Sakkinen wrote:
> > > It's not a "punishment". It's more like that I really have to take the
> > > time to read the prose...
> >
> > The thing about imperative tense is that it was used as a punishment on
> > me once five years ago.  I wrote a quite bad commit message and a senior
> > maintainer told me to re-write it properly and I realized that it was
> > true.  My commit message was bad.  So I wrote a proper commit message.
> > And then he yelled at me, "Can't you follow simple directions and write
> > it in imperative tense like the documentation says?  Are you a
> > shithead?"
>
> Wow :-O I'm totally against name calling or any sort of shittiness like
> that, and all for co-operation. Just told my personal thoughts on the
> matter. I'm sorry that this happened to you.
>
> >
> > So then I swore I would never talk to him again or to anyone who
> > enforced the imperative tense rule.  That has only happened once in the
> > intervening years.  I told the maintainer, "Fine.  Re-write the commit
> > message however you like and give me Reported-by credit."  This was a
> > cheeky response and it made the maintainer enraged.  I guess he thought
> > that my boss would force me to fix the bug or something?  I felt bad for
> > the Intel developer who had to fix my bug instead because I knew that
> > the maintainer was going to be super angry if he gave me reported-by
> > credit so I had put him in a bind.  I almost re-wrote the commit message
> > so that he wouldn't have to deal with that.  Maybe this is how mothers
> > feel when they try to take abuse from an angry husband instead of
> > letting their kids suffer.  But I am a bad mother and I left.
> >
> > My boss would never have forced me to deal with that.  When he left for
> > a different company he said, "Dan, I'm transitioning and XXX is taking
> > over me and I have told him all your weirdness so he is prepared."  And
> > it was a huge comfort to me because I know what my weakness are.
> >
> > You people on this thread all seem super nice.  And you're right that we
> > should always try to be improve every aspect of our craft.
> >
> > When Jarkko talked about people who write too long commit messages, I
> > thought about one developer in particular who writes too long commit
> > messages.  He writes in imperative tense.  He takes everything so
> > seriously and he's never seen a rule without following it.  His patches
> > are always right.  People have told him that his commit messages are bad
> > and too long and those people are right.  But they need to shut up.  The
> > good things that he does and the bad things that he does are all part of
> > the same package.  He can't change and I don't want him to feel anything
> > but welcome.
> >
> > It's hard to be a good kernel developer without being at least slightly
> > obsessive.  Both developers and maintainers are that way.  And I deal
> > with a lot of people and accomodating maintainers you disagree with is
> > part of the job.
> >
> > So long as everyone is kind to each other.  That's the main thing.
>
> I 110% agree with this. I even bookmarked this response :-)

To add: it is super hard and non-trivial issue to keep the balance
between "nice" and "obsessive". If you are too nice, the stuff does not
get fixed. If you are too obssesive, well... then you act like a jerk.
So you have to try to say the truth as clearly as possible, but still
keep the tone constructive. Not always easy to manage.

Personally, I keep bullshit threshold from other people. There are
better and worse days and that does affect your communication. So if
you get one nastier email from someone 9/10 it is better just ignore
the bullshit and focus on matter. Of course when it is a trend, then
it is better to vocally ask for better communication.

BR, Jarkko