[cmake-developers] Review request: extract-cmMessenger branch

Michael Scott michael.scott250 at gmail.com
Tue Feb 2 03:08:18 EST 2016


> Yes. Did you have a close look at the commits? I'm not really sure they are
> correct, and I wonder if you have any thoughts on the first one which
> discusses interface?
I went back and had a closer look at the major changes. I think on the 
whole the cmMessenger class is good and a clear improvement over the 
previous organisation, however I imagine in its current form it would be 
perhaps a bit tricky extending it for other environments (e.g. 
displaying messages as GUI modal notifications), which I understood to 
be one of the overall aims of the branch (correct me if I've 
misunderstood of course though).

I would be tempted to make the following changes to the cmMessenger class:

* Make cmMessenger an abstract class, and have the IssueMessage methods 
pure virtual here, then create an extension containing the current 
implementations called cmConsoleMessenger or something. The name 
cmMessenger suggests a fairly generic concept, but the current 
implementation feels geared around a console, though that's just my 
thoughts.
* Make the IsMessageTypeVisible and ConvertMessageType methods virtual 
and protected, to make it easier to override their behaviour in 
extensions of the base class.
* Change printMessagePreamble to return void
* Rename printMessagePreamble to appendMessagePreamble or something to 
better reflect what its doing, similar change to printMessageText as well
* Move the different parts to displayMessage into individual methods, 
e.g. adding the notes about warning suppression
* Append notes for DEPRECATED_WARNING and DEPRECATED_ERROR message 
types, or remove the current notes, to be consistent

I don't really have any other comments for the other commits. I was 
wondering if it would be possible to move away from accessing the 
cmMessenger class through the cmake / cmMakefile class, but then if we 
do want to slot it new implementations easily I imagine the current 
organisation would facilitate that quite well so it wouldn't be worth 
changing.

Cheers,
Michael


More information about the cmake-developers mailing list