lmi
[Top][All Lists]
Advanced

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

Re: [lmi] 'Οχι day is almost upon us


From: Greg Chicares
Subject: Re: [lmi] 'Οχι day is almost upon us
Date: Sun, 04 Oct 2015 23:37:49 +0000
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Icedove/31.3.0

On 2015-10-04 17:03, Vadim Zeitlin wrote:
> On Sun, 04 Oct 2015 14:40:12 +0000 Greg Chicares <address@hidden> wrote:
> 
> GC> [I refer to 1940-10-28, not the 2015 referendum]
> [referring to the latter in 2005 would have been remarkably prescient]

[I didn't want to ask you, an EU citizen, to vote OXI, without clarifying
that the historical referent was an aggressive imperialistic ultimatum,
as opposed to...whatever]

> GC> Assuming we'll just say "oxi"...Vadim, is the patch below correct?
> 
>  It is correct in the sense that I don't think it does anything wrong, but
> I think more code could be removed: is there any point in keeping the
> validate_print_case_pdf_output() function at all? It doesn't seem to be
> actually testing anything any more,

I supposed it was the only test for "Census|Print case to PDF"; however...

> there is still an occurrence of
> output_pdf_existence_checker in it, but the comment says that we don't
> really care about the tests done by it, so why should we executing this
> code? I would either add some checks for the output PDF or remove this
> function entirely.

Yes, this comment (later in that file) seems to explain it:

// Consider renaming this file to 'wx_test_spreadsheet_output.cpp'
// e.g., because its purpose is to test *spreadsheet* output only.

Grepping for "'i'" easily leads us to 'wx_test_pdf_create.cpp',
which tests "Census | Print case to PDF":

/// Test printing a census document to PDF.
...
LMI_WX_TEST_CASE(pdf_census)
...
    ui.Char('i', wxMOD_CONTROL | wxMOD_SHIFT);  // "Census | Print case to PDF"

so thanks, you're correct: validate_print_case_pdf_output() no
longer serves any purpose and should be removed altogether.

>  Should I make (and test) the patch doing either the former or the latter

The latter ("remove this function entirely") is correct, and I should
be able to get that right.

Please let's do everything in this order:
 - Kim: cast your vote on the original question; presuming "No"...
 - Greg: nuke this whole 'idiosyncrasy_spreadsheet' thing
 - Vadim: review and test my changes, revised as discussed above




reply via email to

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