[Dnsmasq-discuss] patch review request, why
stappers at stappers.nl
Sun Nov 10 23:19:39 GMT 2019
On Sun, Nov 10, 2019 at 01:01:35PM -0800, Kurt H Maier wrote:
> On Sun, Nov 10, 2019 at 11:33:44AM +0100, Patch Submitter wrote:
> > Imagine an open software project gets a patch submitted.
> Substantive patches tend to get more attention. At this point you've
> sent one email for each harmless whitespace character you've proposed to
> delete. Why are you so exercised by this? It's the worst form of
Yes, bikeshedding is an odd way to express "I do care"
Following dnsmasq learnt me that patches get ignored,
at least some patch do need reminders.
Part of being exercised by this is finding the time
between reminders. Other part is getting clear feedback
on submitted patches. Having a "patch seen and is rejected"
means there is no need for sending reminders.
The assumption of harmless white space is wrong. Tooling
such as `git` warns about trailing spaces. Do
rm -f src/000*.patch
git format-patch 04db148...04db148~1
git checkout 6fe436a
patch -p1 < 0001-Fix-crash-on-REFUSED-answers-to-DNSSEC-queries.patch
git diff --check
|src/forward.c:949: trailing whitespace.
|+ if (forward->sentto->edns_pktsz > SAFE_PKTSZ && (forward->flags & FREC_TEST_PKTSZ) &&
|src/forward.c:976: trailing whitespace.
|+ if ((forward->sentto->flags & SERV_DO_DNSSEC) &&
Restore your git tree with
patch -Rp1 < 0001-Fix-crash-on-REFUSED-answers-to-DNSSEC-queries.patch
git checkout master
Petr Mensik wrote wisely:
When I am against forced reformatting like someone here suggested, I
think some easy checks might be done before commiting changes. For
example, git diff would show in red whitespaces on lines without
anything else or after code before end of line. These are not nice and
I would like them removed.
Maybe I should ask for
sed --in-place -e 's/[ \t]*$//' src/*.c
sed --in-place -e 's/^[ \t]*$//' src/*.c
is already a nice cleanup.
The larger plan is having
Code style, ie layout, indent width, is not negotiable.
I like that style, I chose that style and it's not going to change.
documented in something like "clang-format"
While working on that plan was "Remove empty tailing lines"
Leven en laten leven
More information about the Dnsmasq-discuss