lmi
[Top][All Lists]
Advanced

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

[lmi] Pending PRs


From: Vadim Zeitlin
Subject: [lmi] Pending PRs
Date: Sat, 11 Feb 2023 22:31:47 +0100

 Hello,

 Upcoming wx upgrade looks like a good time to clean up some of the old PRs
that you can see (even without a GitHub account) at

        https://github.com/let-me-illustrate/lmi/pulls

(and for each PR you can also see the changes done in it by appending
"/files" to the PR URL -- or by getting the branch name mentioned in
parentheses below, and examining it locally).

 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)

  This is a trivial change removing the only existing use of wxClientDC,
  which is/will be deprecated in wxWidgets soon, in lmi code without
  actually changing anything at all.

- 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
  without this PR compiling it results in multiple occurrences of many
  lines of warnings from this header, making the output of MSVC build quite
  unreadable, so it would be nice to avoid this.


 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.


 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.


 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)

  This was originally done to explain the "mystery" discussed in
  https://lists.nongnu.org/archive/html/lmi/2017-03/msg00045.html
  and I still think it's the right to do but it is hardly critical.

- 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
  during its generation and while such errors shouldn't normally happen at
  all, it still seems like a good idea to do it because having more context
  in the error messages is always useful. If you agree, I could rebase it
  on master to fix the conflict and re-test it.

- 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. This is,
  again, very minor, but if you think it's useful it could be applied, as
  it's a pretty simple change and certainly shouldn't break anything.


 Thanks in advance for looking at this!
VZ

Attachment: pgp_Ibd3rvcH4.pgp
Description: PGP signature


reply via email to

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