Sven Neumann wrote:
> Hi Martin,
> 
> I've found your last commit message quite confusing and so is the FIXME
> that you added to gimpdisplayconfig.c. I'd very much welcome if such
> texts would not be added to the code without prior discussion here.
> 
> Your comment states:
> 
> /* FIXME: If we can get rid of this dependency to the display module,
>  * we will greatly improve the module dependencies in the core, see
>  * devel-docs/gimp-module-dependencies.svg. In particular, we will get
>  * rid of the (transitive) dependency from the core to the UI code
>  */
> #include "display/display-enums.h"
> #include "display/gimpdisplayoptions.h"
> 
> This is quite misleading though, as there is no dependency from the core
> to the UI code. The two files from app/display that are included here
> could as well be moved to app/config. If you look at them, you will see
> that the two files hold nothing but settings. This is config code, it's
> just very closely related to the display module as it deals with display
> configuration that needs to be stored in the gimprc. It's just for
> clarity that these files live in the app/display module. If this leads
> to confusion, we can certainly discuss to move the files.
> 
> If you don't mind, I would like to remove the misleading comment that
> you added. There is nothing that needs to be fixed here. Except perhaps
> for the gimp-module-dependencies.svg graphics. I guess I am
> misunderstanding the purpose of this graphics, but it does not make
> sense for me and as such it is certainly not helpful for potential
> contributors. Perhaps we can try to change the graphics so that it helps
> people who are new to the GIMP code to understand it better?

Hi Sven,

If you find the comment confusing I should explain it in more detail. 
First, I should define the terms I use in the comment. When I say "UI 
code" I am referring to any code that is in modules with a GTK+ 
dependency, which includes the display module. When I say "core", I am 
referring to any code that is in the core modules, modules that don't 
have a GTK+ dependency, such as xcf, text, vectors, core and config. 
With these definitions, which I find reasonable,the core has a 
dependency to the UI code. If you are fine with moving the two files to 
the config module, we should do that. Since this is something that IMHO 
should be fixed, we should not remove the comment.

The purpose of the graphic is to give an overview of how the libraries 
and core modules in the GIMP code base relate to each other. It gives a 
nice overview and I believe it will be beneficial both to new and old 
contributors (I find it useful myself). It can certainly be improved 
further. I do believe however that we should separate a graphic of the 
actual architecture (which the graphic represents, it is generated from 
the GIMP code) from the architecture we want, which should go in a 
separate file.

Regards,
Martin

_______________________________________________
Gimp-developer mailing list
Gimp-developer@lists.XCF.Berkeley.EDU
https://lists.XCF.Berkeley.EDU/mailman/listinfo/gimp-developer

Reply via email to