lmi
[Top][All Lists]
Advanced

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

Re: [lmi] Pending PRs


From: Greg Chicares
Subject: Re: [lmi] Pending PRs
Date: Tue, 14 Feb 2023 23:49:31 +0000
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.3.0

On 2/11/23 21:31, Vadim Zeitlin wrote:
[...]
>  I'm going to separate them in different groups. First one contains simple
> fixes that I'd like to see applied and which, hopefully, shouldn't be
> controversial:
> 
> - Don't use wxClientDC class unnecessarily
>   https://github.com/let-me-illustrate/lmi/pull/222 (dont-use-client-dc)

Cherry-picked. I first tried merging it instead, because IIRC you had
expressed a preference for that in the past, but in that case the
commit message said only that it was a merge from a particular SHA1.
Cherry-picking preserves your commit message.

> - Disable MSVC warning about negating a value of unsigned type
>   https://github.com/let-me-illustrate/lmi/pull/219 (msvc-avoid-neg-warn)
> 
>   We had a discussion about this some time ago and the conclusion was that
>   you didn't want to change this code of u_abs() in unnatural ways

...but OTOH I don't mind suppressing it pragmatically. Cherry-picked.

>  Second group contains PRs related to updating wxWidgets/wxPdfDoc and I
> won't mention them here because I'd like to focus on the other PRs in this
> post. Of course, if you think that updating these libraries should be done
> first, we can do it like this, but I thought that it would be nice to deal
> with the much smaller changes discussed here first -- and then test them
> when testing the new wx version.

Okay, let's consider the other PRs in this message first. We do want to
upgrade wx quite soon in any case because of the file-extension issue.

>  Next there are a couple of PRs proposing optimizations:
> 
> - Only recreate census view columns if really necessary
>   https://github.com/let-me-illustrate/lmi/pull/113 (census-view-columns-opt)
> 
> - Allow faster access to MemberSymbolTable members by index
>   https://github.com/let-me-illustrate/lmi/pull/114 (any_member-opt)
> 
>  Both of them have conflicts currently but I could fix them easily. The
> question is whether it's worth doing it or if I should just close them
> because it seems unlikely that they're going to be applied, after 4 years
> spent sitting there. I rather suspect it's the latter, but please let me
> know if I'm wrong. FWIW I certainly can live without them, it just seems a
> pity that lmi users won't profit from these optimizations.

More likely, they just got forgotten. Clearly we need to test changes
like these before putting them into production. I propose that we
upgrade wx (and all the other libraries) first, then rebase and apply
these changes, and then test everything.

>  Finally there are a couple of minor fixes that could be applied if you'd
> like or just closed otherwise. Those don't (or at least shouldn't) have any
> user-visible effects, so they're arguably even less important than the
> optimizations above:
> 
> - Better fix for USE_SO_ATTRIBUTES=1 build #56 
>   https://github.com/let-me-illustrate/lmi/pull/56 (better-so-fix)

Cherry-picked.

>   This was originally done to explain the "mystery" discussed in
>   https://lists.nongnu.org/archive/html/lmi/2017-03/msg00045.html

[...BTW, that git-bisect would have benefitted from ccache...]

>   and I still think it's the right to do

Cherry-picked.

> - Improve error reporting for PDF generation 
>   https://github.com/let-me-illustrate/lmi/pull/116 (pdf_gen_errors)
> 
>   I don't remember the details about this one any more, but the idea was
>   that we should report the page of the PDF when reporting any errors

Let's revisit and test that after upgrading wx and wxPdfDoc.

> - Show the maximum time and the standard deviation in the timers results
>   https://github.com/let-me-illustrate/lmi/pull/150
> 
>   You asked to improve this output a couple of years ago and this was done
>   in this PR, the last comment there shows the new output format.

It'd be good to show σ, but IIRC I wanted to reconsider the format.
Let's leave this one in abeyance for now.

I'm testing all the cherry-picks above now, and will push them soon
as long as all the tests succeed.



reply via email to

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