lmi
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [lmi] PATCH: C++20 and clang


From: Vadim Zeitlin
Subject: Re: [lmi] PATCH: C++20 and clang
Date: Tue, 20 Apr 2021 02:56:50 +0200

On Tue, 20 Apr 2021 00:38:45 +0000 Greg Chicares <gchicares@sbcglobal.net> 
wrote:

GC> Indeed I do have 'origin' configured to point to savannah.
GC> But I hadn't pulled from savannah since yesterday, so I
GC> was probably comparing your branch to local master.

 Yes, but I still don't understand why there was no mention of
origin/master update in "git fetch --all" output. In any case, 

GC> I believe I've merged everything now.

 Thanks a lot! Unfortunately the first commit of this series, i.e.

https://github.com/let-me-illustrate/lmi/pull/179/commits/9a4888100bbf4029c72fc9c598122079674dcb9f

seems to be missing from master. I don't think it was intentional and while
it doesn't break the CI build any longer (because later commits disabled
the warning this code generated with clang in C++20 mode), I believe it
would still be nice to apply it. Could you please do it?

GC> I did some fixups to make 'make $coefficiency check_concinnity'
GC> succeed.

 Oops, really sorry about this, I should have rerun it. But I don't
understand how did the CI builds pass, they're supposed to check for
this... oh, now I see: the problem was indeed detected, see

https://github.com/let-me-illustrate/lmi/runs/2382487360?check_suite_focus=true#step:22:25

but apparently "make check_concinnity" doesn't exit with error even when it
does detect problems. This is really unexpected and counter-intuitive.
Would you consider accepting patches changing this?

GC> >  In any case, the commits that are part of this PR are the first 6 commits
GC> > in the list above (they're also those that appear on the web page if you
GC> > open the URL above in the browser).
GC> From your previous messages, it seems that you took extra
GC> pains to present this patchset in various ways to help me.
GC> That's appreciated, but it's not really necessary as I
GC> don't take advantage of all the diversity. Let me explain
GC> what I did here, because it may save you time in future.

 My explanations are just meant to be complimentary to the commit messages
and also mention things that I didn't do even thought they might be worth
doing. I.e. I do understand that you apply the commits locally and not
using web UI, the links to the commits on GitHub are just for reference. I
thought that having the extra information in the post could be useful,
compared to just having the commit messages themselves, but if you think
this is too much and that commit messages are enough, I could avoid writing
these additional explanations, of course.

 Please let me know what would you prefer in the future,
VZ

Attachment: pgpsgrCFgrnA6.pgp
Description: PGP signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]