Cactus: Add CCTK_VINFO, CCTK_VERROR, CCTK_VWARN

Comments (10)

1. 

   Roland Haas

   • removed comment

   As far as I can tell this introduces (among other unrelated changes) a set of macros such that eg. CCTK_VINFO("a %d", 42) gets turned into CCTK_VInfo(CCTK_THORNSTRING, "a %d", 42) ie. one saves on the CCTK_THORNSTRING. The commit thus makes the CCTK_UPPERCASE macros more orthogonal to the corresponding CCTK_LowerCase functions since the difference between the two is now only that one set includes CCTK_THORNSTRING, __LINE__, __FILE__ and the other one does not.

   The patch is lacking documenation updates for the new macros and should not be applied without.

   Comments:

   • please provide documentation for the macros in CCTK_Reference.tex
   • there are extra changes that reformat lines that are not otherwise touched by the commit, I would leave those out if possible

   Fine with me as long as the first point is taken care of (the second one is not crucial, but documentation is).

   • 2016-07-25T11:47:27+00:00
2. 

   Roland Haas

   • removed comment

   This fails to compile code like this for me:

   CCTK_VINFO("Message without formatting");
   

   I suspect this could be cured by using

   #define CCTK_VINFO(...) CCTK_VInfo(CCTK_THORNSTRING,__VA_ARGS__)
   

   ie only having the VA argument so that the error only shows up if no argument is passed to VINFO at all.

   • 2017-01-19T18:45:40+00:00
3. 

   Ian Hinder

   • changed status to open
   • assigned issue to
   • removed comment

   Related: #2005 is a report of code having been committed that depends on the proposed code in the current ticket which unfortunately breaks the build.

   • 2017-02-04T07:43:24+00:00
4. 

   Roland Haas

   • changed status to open
   • removed comment

   • 2017-02-06T11:53:12+00:00
5. 

   Frank Löffler

   • removed comment

   Some points: - I don't link other changes in the same diff. It's ok for now, but please don't mix changes even if it just changes upper case / lower case of something unrelated - If I read the changes to the documentation correctly, places with, e.g., CCTK_VInfo are replaced by CCTK_VINFO. Isn't CCTK_VInfo still there, usable, and should stay mentioned, like in "Note that the routines \texttt{CCTK_VError()} and \texttt{CCTK_VWarn()} can only be called from C" - Seeing the diff of the pdf in github mixed with the other changes is annoying. I know this decision was made to keep the diffs to the pdfs small, but it is really annoying here.

   • 2017-02-06T12:37:22+00:00
6. 

   Roland Haas

   • removed comment

   • If I read the changes to the documentation correctly, places with, e.g., CCTK_VInfo are replaced by CCTK_VINFO. Isn't CCTK_VInfo still there, usable, and should stay mentioned, like in "Note that the routines \texttt{CCTK_VError()} and \texttt{CCTK_VWarn()} can only be called from C"

   I noticed that as well and was a bit upset initially. However preferring CCTK_VINFO over CCTK_Vinfo is along the lines of preferring CCTK_INFO over CCTK_Info which has always been the case. So my estimate in the review was that this is a valid change that preserves consistency in the documentation and the recommended functions.

   The pdf diffs are annoying I agree. I don't know of a way on top of my head to avoid them (declaring the pdf's binaries likely will counteract the initial idea of making git create diffs for them in the first place). I don't really see them as any different from the diffs of configure though which is also an auto-generated file and contains lots of changes each time its source file configure.in is changed.

   • 2017-02-06T13:02:12+00:00
7. 

   Roland Haas

   • removed comment

   (this was stuck in queue and apparently I forgot to hit "submit", sorry).

   Please apply.

   The commit makes the code build again. Some issues in the documentation remain:

   • it replicates the information in the documentation section for CCTK_VWARN in CCTK_VWarn. One should be shortened to refer (with a link) to the other
   • the code change mixes new features in one section of the file with code reformatting in another. This makes it hard to review.

   • 2017-02-06T13:02:49+00:00
8. 

   Roland Haas

   • changed status to open
   • removed comment

   • 2017-02-06T13:03:22+00:00
9. 

   Erik Schnetter reporter

   • changed status to resolved
   • removed comment

   These change have been applied via commit d05d608484f3b6e74a53f42693efab48343f54e7.

   • 2017-02-08T10:09:12+00:00
10. 

    Roland Haas

    • edited description
    • changed status to closed

    • 2019-02-21T19:49:54+00:00
11. Log in to comment