Re: [Dnsmasq-discuss] patch review request, why

2019-11-10 Thread Kurt H Maier
On Mon, Nov 11, 2019 at 12:19:39AM +0100, Geert Stappers wrote:
> 
> The assumption of harmless white space is wrong. Tooling
> such as `git` warns about trailing spaces. Do

This is bizarre logic.  What makes git an authority on code formatting?
Who elected the git maintainers into the style police?  If git began
issuing warnings about using upper-case characters would you reformat
all your source code?

Tools support the developers, not the other way around.

khm

___
Dnsmasq-discuss mailing list
Dnsmasq-discuss@lists.thekelleys.org.uk
http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss


Re: [Dnsmasq-discuss] patch review request, why

2019-11-10 Thread Geert Stappers
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
> bikeshedding.

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

to get
|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.
(http://lists.thekelleys.org.uk/pipermail/dnsmasq-discuss/2019q3/013368.html)


Maybe I should ask for
  sed --in-place -e 's/[ \t]*$//' src/*.c
but
  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.
(http://lists.thekelleys.org.uk/pipermail/dnsmasq-discuss/2019q3/013318.html)
and
  I like that style, I chose that style and it's not going to change.
(http://lists.thekelleys.org.uk/pipermail/dnsmasq-discuss/2019q4/013414.html)
documented in something like "clang-format"
(http://lists.thekelleys.org.uk/pipermail/dnsmasq-discuss/2019q4/013417.html)

While working on that plan was "Remove empty tailing lines"
(http://lists.thekelleys.org.uk/pipermail/dnsmasq-discuss/2019q4/013418.html)
revealed.



Groeten
Geert Stappers
-- 
Leven en laten leven

___
Dnsmasq-discuss mailing list
Dnsmasq-discuss@lists.thekelleys.org.uk
http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss