For large scale patches: definitely. It’s often easier for a project to let its own experts write their own 100 line patch than it is to review someone else’s 100 line patch.
In this instance: it’s a changelog entry and a patch to a C file (8 insertions(+), 1 deletion(-)). Remove the printed warning and you’re back to 5 insertions. Furthermore, the patch has already been merged upstream.
Redhat is free to choose what patches they do and do not accept and the low quality Reddit outrage posts in the Gitlab issue sure doesn’t make things any better, but this is quite a silly thing to refuse to merge.
A patch contains more than the changes: it contains the commit message. In open source projects, and in particular in CVE fixes, the commit message can indeed be quite descriptive. It needs to be!
You’re still right, though. But I like to think professionals are able to verify the changes with the high-quality commit message—possibly in less time than investigating the issue themselves.
For large scale patches: definitely. It’s often easier for a project to let its own experts write their own 100 line patch than it is to review someone else’s 100 line patch.
In this instance: it’s a changelog entry and a patch to a C file (8 insertions(+), 1 deletion(-)). Remove the printed warning and you’re back to 5 insertions. Furthermore, the patch has already been merged upstream.
Redhat is free to choose what patches they do and do not accept and the low quality Reddit outrage posts in the Gitlab issue sure doesn’t make things any better, but this is quite a silly thing to refuse to merge.
A patch contains more than the changes: it contains the commit message. In open source projects, and in particular in CVE fixes, the commit message can indeed be quite descriptive. It needs to be!
You’re still right, though. But I like to think professionals are able to verify the changes with the high-quality commit message—possibly in less time than investigating the issue themselves.
How did they submit changes to only one file? Did they not write a test for it? Sounds like a dodgy patch if it doesn’t have a test
The project’s tests aren’t exactly exhaustive.I don’t think anyone expected them to add any tests.