This article mainly introduces personal experiences in the community and some interesting anecdotes observed from subscription emails.
# Krzysztof Wilczyński
I have also been deeply affected by this maintainer, not just me, but also my colleagues in the company, and over 100 to 500 people in the community who have experienced patches being commented on by this big shot.
Next, I will discuss the processes involved.
# First, Niklas Cassel
Niklas has successfully submitted 150 patches in the Linux kernel..
He is not a maintainer, but an ordinary contributor. Initially, he was very active in the PCIe domain, and my first patch was to fix a bug in his previously submitted code.
Later, I wanted him to provide some review comments on some patches, but he replied that he was not a maintainer.
There was a bit of friction between him and Krzysztof that started with one of my patches. The reason was that when I sent the next version, the main change was to add the maintainer’s review tag: Reviewed-by,
but the subject version of the patch did not increment by 1, instead, it was RESEND + old version number. Krzysztof felt that it was not acceptable to RESEND + old version number, while Niklas thought it was fine because the patch itself had not changed, only the review tag was added, mainly to prevent the reviewer’s tag from being discarded, which would waste the time of others who had already reviewed it.
The key point is: 【 Here I will attach the original text and the translation: Youdao Translation 🙂 】
Niklas reminded Krzysztof in the email that there was an error code in the email name.
Hello, Krzysztof, for your reference: Your name appears incorrectly in my inbox. Sender: Krzysztof Wilczyński <your email address> perhaps because: Content-Type: text/plain; charset=US-ASCII instead of UTF-8?
Having reviewed the differences, I believe these changes are fine, but I strongly feel that if the actual code has been modified (rather than just correcting some minor syntax errors in the commit message), then the (unwritten?) rule is that the submitter should add a: [Person: description of the differences from the original submission] after the "Signed-off-by" line of the original submission. For example, if there are errors in the changed lines, then the original author should not be unfairly blamed for code that he did not write. Only modifying the commit log cannot introduce functional changes, but as long as the code/differences are modified, it is possible (intentionally or unintentionally) to introduce functional changes, so there is a significant difference between these two situations (in my humble opinion).
Sorry, I did forget to add this. Your reminder is really good. That said, a simple prompt or reminder message would be enough. There is no need to adopt a condescending tone, nor to preach. You have been doing this for a while, and if you continue like this, I will have no choice but to start ignoring your submissions, as I do not have time to deal with this negative confrontational attitude.
I have read the reply several times, but to be honest, I really do not understand what this is all about. I just want to say that if the actual code has changed during application, then I think it is necessary to take one of the following two actions: 1) add relevant content in the corresponding thread (for example, in the "applied" message), 2) add a [User:] annotation after the "standard operating procedure". As for merely modifying the commit log, I personally believe that the importance of points 1) and/or 2) is much lower. I do not mean to preach, I just want to clarify the logical basis of my viewpoint. It surprises me that our only recent communication was about the "resend" tag, at which time I also expressed my personal opinion. But in my view, that was also a reasonable discussion. When performing a resend operation (as long as the code and commit log have not changed), it seems somewhat counterproductive to add tags like "Reviewed-by" and "Tested-by", as the time spent by testers/reviewers would be wasted.# Second, Ilpo Järvinen
Ilpo has successfully submitted 575 patches in the Linux kernel.
This patch changes the print message from two lines to one line.
I do not know if there is still such a thing as a "trivial patch maintainer", so I will pull this. I guess this must be somewhat troubling for you. That is to say, I... your expertise and time could have been used elsewhere.Then this patch was directly accepted and placed into a temporary branch.
Doing such patches does not take long, and I do not deliberately look for such things, nor do I focus on them. It is just that when I use my expertise in different ways, I also encounter these simple issues. You can check the recent tracking records I found, such as real concurrency issues, so I hope you can provide some trivial patches here and there; -)
We love your work! The attention to detail is unparalleled, especially. :) Regarding the topic of trivial patches... there is a big debate about trivial patches in the community, per: https://lore.kernel.org/all/[email protected]/t/ there are many viewpoints... ultimately, as usual, it depends on the maintainer. Thanks again for everything you do! Thank you!# Third, the author (me)
I need to take some time to investigate this issue, so I chose to do something else. Recently, I have been very impatient during debates and would say, "I have seen this elsewhere." I found vulnerabilities elsewhere, so I feel I can introduce this content. They?
Are there really two people reviewing these minor changes? Really? Moreover, what exactly did their review check? This is obviously wrong—failing to follow DTS coding style, so what does such a review mean? What does it really mean?Summary: Don’t add DTS, DT binding documentation if you don’t believe in evil, or you will be beaten up.
# Fourth, a colleague from the company
This is useless duplicate information. Twice. You can easily find the problem yourself instead of constantly bothering the reviewers with these trivial matters. NAK, please remember not to waste the reviewer's time. > You see it twice, each pin controller once. By the way, as you suggested earlier, we will print the value of ret in the error message. > If I missed any information, please remind me. Thank you for still ignoring my second comment.The maintainer of pinctrl also feels somewhat helpless about being criticized.
Yes, I understand this idea. A typical symptom of a non-probe pin control driver is: for example, the system cannot mount the root filesystem because some pins connected to the eMMC are not correctly multiplexed. People will eventually discover this problem; it just makes the issue manifest faster when printed out. And this kind of error is quite common (at least for me, but I am not the best developer...)Finally, I advise everyone to avoid mentioning patches unless you can handle the backlash.
Welcome to follow the author’s WeChat public account: irregularly publish PCIe-related knowledge points and some interesting dynamics in the Linux kernel community.