[PATCH] D24040: codechecker tool core

[Laszlo Nagy via cfe-commits]

rizsotto.mailinglist added a comment.

In https://reviews.llvm.org/D24040#532718, @o.gyorgy wrote:

> In https://reviews.llvm.org/D24040#530293, @rizsotto.mailinglist wrote:
>
> > Gyorgy and the ericsson team, thanks for doing this. very good job! good targeted functionality. i don't want to underestimate the complexity it requires, but to me this is a giant code. i do miss the explanation of the overall functional description what a module does and how it relates to other modules. i found a gap between the high level overview and the code comments.
>
>
> Would that help If update architecture.md documentation (which gives a high level overview) to be more in sync with the source code, with additional comments and descriptions in the modules?


i prefer python docstrings instead of the markdown files. found that the `__init__.py` files are empty. those are good candidates for a higher overview of the package architecture. now module headers are one liners most of the places, which is a missed opportunity to document the connections between the modules. method docstrings are non descriptive enough. (echoing back the function names with spaces.) would read about the usage of the method, contract between the caller and the callee, or the implementation difficulties.

> > generally speaking i found this code under documented and a bit verbose. the comments in the code is not really paint the picture that i need in order to fix a bug, or implement a feature. and it might only be my personal taste, but found that creating types (or classes this case) in a dynamically typed script language, it makes the code very verbose while it does not implement much.

> 

> 

> We use classes to handle and setup environment, build, analyzer and other configurations. In the infrastructure this makes available to easily introduce new analyzers and handle the output of them. Multiple output handling is supported for each analyzer (print to stdout, store to database).


yes, i got that you were using OO. and try to re-use code this way... my point is, this abstraction, in this language, ends up to have an abstract class with empty methods, a couple of implementations and the user of the abstract class, which enjoys the polymorphism. to understand a single implementation i have to read (at least) three different modules/files. it's very fragmented to me... my proposal would be, to write util methods. implement the different analyzer commands as a combination of the util methods. this abstraction fits better to a script language. it also make your code more dense, less verbose, easier to maintain, test or reason about... those was my two cents.


https://reviews.llvm.org/D24040