[Libreoffice] Script to find undocumented classes

2010-11-30 Thread Miklos Vajna
Hi,

I'm attaching a patch that adds a script to the scratch directory of a
build repo that tries to find undocumented classes using doxygen. It's
meant to be invoked in a subdirectory of the source code, so in case one
is familiar with Calc, she can run it under sc/ and add doxygen comments
to the classes she know, etc.

I'm intentionally not using 'make docs', because the target here is to
allow quick iteration, ie. to run 'find-undocumented-classes.sh -q' in a
subdirectory, add a few comments, run again, etc.

Does the idea sounds sane, OK to push?

Thanks.
From 0fd0d400e3a64cb9619a207868c835d18acce162 Mon Sep 17 00:00:00 2001
From: Miklos Vajna 
Date: Mon, 22 Nov 2010 14:33:57 +0100
Subject: [PATCH] Add script to find undocumented classes

---
 scratch/find-undocumented-classes.sh |   26 ++
 1 files changed, 26 insertions(+), 0 deletions(-)
 create mode 100755 scratch/find-undocumented-classes.sh

diff --git a/scratch/find-undocumented-classes.sh 
b/scratch/find-undocumented-classes.sh
new file mode 100755
index 000..58a133b
--- /dev/null
+++ b/scratch/find-undocumented-classes.sh
@@ -0,0 +1,26 @@
+#!/bin/bash
+
+# finds undocumented classes in the current directory (recursive)
+
+filter=
+quiet=n
+if [ "$1" = "-q" ]; then
+filter=">/dev/null"
+quiet=y
+fi
+
+doxygen=$(mktemp -d)
+eval doxygen -g $doxygen/doxygen.cfg $filter
+sed -i "/HTML_OUTPUT/s|html|$doxygen/html|" $doxygen/doxygen.cfg
+sed -i '/GENERATE_LATEX/s/= YES/= NO/' $doxygen/doxygen.cfg
+sed -i '/RECURSIVE/s/= NO/= YES/' $doxygen/doxygen.cfg
+eval doxygen $doxygen/doxygen.cfg $filter 2> $doxygen/errors.txt
+if [ "$quiet" == "n" ]; then
+echo
+echo "The following classes are undocumented:"
+echo
+fi
+cat $doxygen/errors.txt|grep 'Warning: Compound.*is not documented'
+rm -rf $doxygen
+
+# vim:set shiftwidth=4 softtabstop=4 expandtab:
-- 
1.7.3.2



pgpH5wsBDaPkn.pgp
Description: PGP signature
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: [Libreoffice] Script to find undocumented classes

2010-11-30 Thread Thorsten Behrens
Miklos Vajna wrote:
> Does the idea sounds sane, OK to push?
> 
Totally! Would you maybe also be interested to work on an improved
start page / index, e.g. like this here:

http://www.informatik.uni-hamburg.de/~meine/hg/vigra/file/bf402d01f821/docsrc/index.dxx

(see
http://www.informatik.uni-hamburg.de/~meine/hg/vigra/file/bf402d01f821/include/vigra/matrix.hxx
for how defgroup / ingroup etc. could be used)?

Additionally, I think most classes don't necessarily need detailed
docs for all methods in the first place (which may also hurt later
merging from OOo), but would already benefit from a two-line
"mission statement" at class level (of course plus some module-level
overview of "what's inside").

Cheers,

-- Thorsten


pgppVNIlYFzxW.pgp
Description: PGP signature
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: [Libreoffice] Script to find undocumented classes

2010-11-30 Thread Lubos Lunak
On Tuesday 30 of November 2010, Thorsten Behrens wrote:
> Additionally, I think most classes don't necessarily need detailed
> docs for all methods in the first place (which may also hurt later
> merging from OOo), but would already benefit from a two-line
> "mission statement" at class level (of course plus some module-level
> overview of "what's inside").

 I beg to differ. After having years of experience using a nice, intuitive and 
well-documented APIs (Qt,KDE), and being used to that, I sometimes rather 
suffer getting familiar with this codebase. Most APIs are not documented at 
all (or at most poorly, or in German, which is about the same in practice for 
many people). This is futher made worse by some APIs not being very intuitive 
(cryptic abbreviations, unclear naming, obsolete idiosyncracies, duplication, 
basic things being needlessly complicated).

 This could be a significant factor for new possible contributors. While 
patches removing dead code or similar certainly help too, the codebase can 
move forward only by people writing new code, and that requires understanding 
of the existing code.

 So I find any suggesting that docs are not necessary (or valueing merging 
higher) to be eventually shooting ourselves in the foot.

-- 
 Lubos Lunak
 l.lu...@suse.cz
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: [Libreoffice] Script to find undocumented classes

2010-11-30 Thread Miklos Vajna
On Tue, Nov 30, 2010 at 04:21:08PM +0100, Thorsten Behrens 
 wrote:
> Miklos Vajna wrote:
> > Does the idea sounds sane, OK to push?
> > 
> Totally!

OK, pushed.

> Would you maybe also be interested to work on an improved
> start page / index, e.g. like this here:
> 
> http://www.informatik.uni-hamburg.de/~meine/hg/vigra/file/bf402d01f821/docsrc/index.dxx
> 
> (see
> http://www.informatik.uni-hamburg.de/~meine/hg/vigra/file/bf402d01f821/include/vigra/matrix.hxx
> for how defgroup / ingroup etc. could be used)?

Hm, sure - but I guess that in general needs a general knowledge of at
least a whole module, which is not really the case for me. Or you mean
a group for each subdirectory, which is probably something I can do? :)

> Additionally, I think most classes don't necessarily need detailed
> docs for all methods in the first place (which may also hurt later
> merging from OOo), but would already benefit from a two-line
> "mission statement" at class level (of course plus some module-level
> overview of "what's inside").

Yes, I think it's more important to have a mission statement for a lot
of classes than a few fully documented ones. :)

That's what the script does - normally doxygen emits tons of warnings
and this way it's easy to search for classes where there isn't a single
line of doxygen comment at all.

OTOH in the long run I agree with Lubos that it would be nice to have
methods documented as well, unless that makes merging really hard. (I
don't think it makes merging harder than comment cleanup does.)


pgpmM6ovrCaE8.pgp
Description: PGP signature
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: [Libreoffice] Script to find undocumented classes

2010-11-30 Thread Thorsten Behrens
Lubos Lunak wrote:
> On Tuesday 30 of November 2010, Thorsten Behrens wrote:
> > Additionally, I think most classes don't necessarily need detailed
> > docs for all methods in the first place (which may also hurt later
> > merging from OOo), but would already benefit from a two-line
> > "mission statement" at class level (of course plus some module-level
> > overview of "what's inside").
> 
>  I beg to differ. After having years of experience using a nice, intuitive 
> and 
> well-documented APIs (Qt,KDE), and being used to that, I sometimes rather 
> suffer getting familiar with this codebase. Most APIs are not documented at 
> all (or at most poorly, or in German, which is about the same in practice for 
> many people). This is futher made worse by some APIs not being very intuitive 
> (cryptic abbreviations, unclear naming, obsolete idiosyncracies, duplication, 
> basic things being needlessly complicated).
>
Hi Lubos,

adding documentation for what you describe doesn't help much, if at
all. Core API is reasonably documented, see offapi/udkapi/sal/basegfx.

The most egregious cases of hard-to-grasp methods surely live in the
applications cores, use lots of weird abbreviations - and to
document their behaviour succinctly would prolly require as much
prose as there's code already inside them (i.e. multiple pages).

For me, a cleanly designed class with one and only one task (and a
proper mission statement, maybe - superfluous even for something 
like "String"), does not need much method documentation.
Suitably-chosen method names, especially if they don't take 10
parameters, and only work for exactly 42 combinations of them, are
almost self-documenting.

I simply refuse the notion that adding superficial documentation to
crappy api gets us anywhere - which does not rule out storing
hard-won knowledge of class purpose, inner workings, important pre-
or postconditions at times - and if it makes sense, as method- or
class-level doxygen documentation.

But to really fix the problem you mention, I'm afraid only
large-scale refactoring helps. And wrong documentation surely is 
worse than no documentation.

(sorry for the rant, that struck a chord. Also, I'm clearly with you
that larger parts of vcl & [sv]tools public headers are in dire need
of method-level documentation. I'd even suggest to start there, that
code does not change much)

Cheers,

-- Thorsten


pgpxbxY0c6BKk.pgp
Description: PGP signature
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: [Libreoffice] Script to find undocumented classes

2010-12-01 Thread Sebastian Spaeth
On Tue, 30 Nov 2010 18:50:17 +0100, Lubos Lunak  wrote:
> On Tuesday 30 of November 2010, Thorsten Behrens wrote:
> > Additionally, I think most classes don't necessarily need detailed
> > docs for all methods in the first place (which may also hurt later
> > merging from OOo), but would already benefit from a two-line
> > "mission statement" at class level (of course plus some module-level
> > overview of "what's inside").
> 
>  I beg to differ.

>  This could be a significant factor for new possible contributors. While 
> patches removing dead code or similar certainly help too, the codebase can 
> move forward only by people writing new code, and that requires understanding 
> of the existing code.

I am with Lubos here I have to say. I do see Thorsten's point on that
wrong documentation is worse than no documentation, and fixing wrong
APIs and function names would be preferred. I also see how it makes
merging worse. 

BUT, as a newcomers, I was very much overwhelmed with millions of lines
of code, many of which were not documented at all. And as a newcomer you
don't even know what is a core API and what is a seldomly used
thing. 

People who are not intimate with but rather intimidated
by the code base, do need these docs to steer around and understand the code.

Being able to look up in doxygen what a "bFull" parameter implies
and which effect it has in a function, would be a real boon. Let me give
a random real world example that I just pulled up by searching the code
base for a bFull parameter :):

In /libs-core/editeng/inc/editeng/unofored.hxx we define:
QuickFormatDoc( BOOL bFull=FALSE );

What does bFull mean? Not so quick? What portions will be formatted if
this is FALSE? Looking at the function it either calls
 pImpEditEngine->FormatFullDoc(); 
 or 
 pImpEditEngine->FormatDoc();

What the heck is the difference between those functions? Now I have to
go another layer deeper to those 2 (both undocumented!) functions.

All that FormatFullDoc does:

  for ( sal_uInt16 nPortion = 0; nPortion < GetParaPortions().Count(); 
nPortion++ )
GetParaPortions()[nPortion]->MarkSelectionInvalid( 0, 
GetParaPortions()[nPortion]->GetNode()->Len() );
  FormatDoc()

Excuse me, what does the stuff before we end up in FormatDoc() actually
do? It must modify the document somehow as a sideeffect,because we call
FormatDoc without any parameter. And it seems to mark some selections as
invalid. So perhaps bFull=FALSE only works on selected text?

For that I need to dig into what GetParaPortions and ParaPortions
actually are and do, which is an *undocumented class* implemented here: 
libs-core/editeng/source/editeng/editdoc2.cxx.

I'd give up at this point, because after reading that much code, I had
forgotten what I wanted to do in the first place :). A simple docstring
in QuickFormatDoc, such as
/**
 * param bFull determines whether we need to reflow the whole document
 or only the pieces that are visible on the screen.
 */
would have saved me much time, and I could actually have improved some
code rather becoming a frustrated opengrok hunter..
(note this is a bullshit comment, as I *still* don't know what bFull
really does :)).

Sebastian


pgpAGSX9awKLR.pgp
Description: PGP signature
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: [Libreoffice] Script to find undocumented classes

2010-12-01 Thread Tor Lillqvist
> After having years of experience using a nice, intuitive 
> and well-documented APIs (Qt,KDE), 

But surely you should realize the causality here? It is exactly your years of 
experience with them that makes Qt and KDE seem nice and intuitive.

It is extremely hard, probably impossible, to come up with an objective way to 
indicate what is intuitive. As the old saying goes, the only intuitive thing 
(for a mammal) is the nipple. Everything else is learned.

The same argumentation is seen over and over again in all areas of software... 
"I have used Photoshop for years, it is so intuitive, GIMP sucks." "I have used 
Windows for years, it is so intuitive, Linux sucks." (Intentionally just saying 
"Linux" here because people who make that claim hardly know which FLOSS desktop 
environment they are referring to.) "I have used Fortran for years, it is so 
intuitive for numerical programming."

I am sure it is not hard to find people who think the OOo code APIs are 
elegant, consistent and intuitive. And they would be equally right.

> So I find any suggesting that docs are not necessary (or valueing merging 
> higher) to be eventually shooting ourselves in the foot.

Personally I think it would be very nice if source files had just a few lines 
of comment telling what they are about, from a very high perspective. Every 
time I am searching for the implementation of some functionality in the OOo/LO 
codebase, I find myself looking at source files with vaguely correct sounding 
file names (and vaguely correctly sounding class names inside), but no clue if 
it actually is what I am looking for.

(Yeah, now that we have forked off the codebase, I of course should take up the 
habit of adding such a few comment lines every time I find out what the code in 
some particular source file does, especially in cases where one can easily be 
mislead and spend hours looking at irrelevant code before realizing...)

--tml


___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: [Libreoffice] Script to find undocumented classes

2010-12-01 Thread Thorsten Behrens
Sebastian Spaeth wrote:
> What does bFull mean? Not so quick? What portions will be formatted if
> this is FALSE? Looking at the function it either calls
>  pImpEditEngine->FormatFullDoc(); 
>  or 
>  pImpEditEngine->FormatDoc();
> 
> What the heck is the difference between those functions? Now I have to
> go another layer deeper to those 2 (both undocumented!) functions.
> 
> All that FormatFullDoc does:
> 
>   for ( sal_uInt16 nPortion = 0; nPortion < GetParaPortions().Count(); 
> nPortion++ )
> GetParaPortions()[nPortion]->MarkSelectionInvalid( 0, 
> GetParaPortions()[nPortion]->GetNode()->Len() );
>   FormatDoc()
> 
> Excuse me, what does the stuff before we end up in FormatDoc() actually
> do? It must modify the document somehow as a sideeffect,because we call
> FormatDoc without any parameter. And it seems to mark some selections as
> invalid. So perhaps bFull=FALSE only works on selected text?
> 
> For that I need to dig into what GetParaPortions and ParaPortions
> actually are and do, which is an *undocumented class* implemented here: 
> libs-core/editeng/source/editeng/editdoc2.cxx.
> 
> I'd give up at this point, because after reading that much code, I had
> forgotten what I wanted to do in the first place :). A simple docstring
> in QuickFormatDoc, such as
> /**
>  * param bFull determines whether we need to reflow the whole document
>  or only the pieces that are visible on the screen.
>  */
>
Hi spaetz,

see? That's what I meant, documentation for most of the higher-level
methods is either

a) superficial
b) so much prose that you're better off debugging the code in the
   first place

(bFull has *a lot* of side effects, and no, I did not bother to
research all of them for the while)

> would have saved me much time, and I could actually have improved some
> code rather becoming a frustrated opengrok hunter..
> (note this is a bullshit comment, as I *still* don't know what bFull
> really does :)).
>
Having a ctags and/or idutils index setup & integrated into your
editor greatly speeds up jumping around in the codebase. I'd consider
it essential for any productive work with LibO.

Cheers,

-- Thorsten


pgpoX6fSo7N6y.pgp
Description: PGP signature
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: [Libreoffice] Script to find undocumented classes

2010-12-01 Thread Lubos Lunak
On Wednesday 01 of December 2010, Tor Lillqvist wrote:
> > After having years of experience using a nice, intuitive
> > and well-documented APIs (Qt,KDE),
>
> But surely you should realize the causality here? It is exactly your years
> of experience with them that makes Qt and KDE seem nice and intuitive.
>
> It is extremely hard, probably impossible, to come up with an objective way
> to indicate what is intuitive. As the old saying goes, the only intuitive
> thing (for a mammal) is the nipple. Everything else is learned.
>
> The same argumentation is seen over and over again in all areas of
> software... "I have used Photoshop for years, it is so intuitive, GIMP
> sucks." "I have used Windows for years, it is so intuitive, Linux sucks."
> (Intentionally just saying "Linux" here because people who make that claim
> hardly know which FLOSS desktop environment they are referring to.) "I have
> used Fortran for years, it is so intuitive for numerical programming."
>
> I am sure it is not hard to find people who think the OOo code APIs are
> elegant, consistent and intuitive. And they would be equally right.

 Well, no, sorry.

 First of all, while most of my experience comes from working with Qt/KDE, I 
usually worked on the lower level parts of it, making me often look at other 
software, be it libs like fontconfig, Xlib, polkit, or various non-KDE 
software. So I do have quite some experience figuring out unknown code.

 Second, I think it is possible to measure how good API/documentation is, to 
some extent. You can compare how extensively and well it is documented (sure, 
there are some exceptions like polkit being very extensively and rather 
unhelpfully documented, but generally it's rather hard to beat something like 
e.g. [1]). You can look at some code and see whether you can guess the 
meaning even without being familiar with it ("const SwFmtFrmSize& rLSz = 
pLineFmt->GetFrmSize();" - huh? I still need to decipher this one and I've 
been told the meaning at least 3 times). You can look at the documentation 
and see how quickly you find a way to do something you need. And so on.

 I'm not going to dwell much on showing that Qt/KDE APIs are easier to use, 
because quite frankly I find that obvious, you can just ask anybody around or 
try it yourself. They are simply designed that way and there are certain 
requirements, while it seems LibO doesn't have that. I may be biased because 
of familiarity, but the difference is simply huge. And BTW, [2] is a very 
interesting read.

 I don't mean to bash anybody for this, but this is simply the biggest 
impression with LibO that I have - that it's awfully difficult to get into 
it. The combination of massive codebase and poorly documented (and often 
cryptic) APIs is just overwhelming. I may have time to get over it, but does 
that mean we're not interested in those possible contributors who wouldn't 
have this much time and guidance? Speaking of which, since the opening of 
LibO, how many of those new contributors have contributed something that 
requires at least some understanding of the codebase (i.e. not counting dead 
code removal, comment translations or similar)?
   
[1] http://doc.qt.nokia.com/4.7/qstring.html
[2] http://doc.trolltech.com/qq/qq13-apis.html

> > So I find any suggesting that docs are not necessary (or valueing merging
> > higher) to be eventually shooting ourselves in the foot.
>
> Personally I think it would be very nice if source files had just a few
> lines of comment telling what they are about, from a very high perspective.
> Every time I am searching for the implementation of some functionality in
> the OOo/LO codebase, I find myself looking at source files with vaguely
> correct sounding file names (and vaguely correctly sounding class names
> inside), but no clue if it actually is what I am looking for.
>
> (Yeah, now that we have forked off the codebase, I of course should take up
> the habit of adding such a few comment lines every time I find out what the
> code in some particular source file does, especially in cases where one can
> easily be mislead and spend hours looking at irrelevant code before
> realizing...)

 I think that would be very helpful.

-- 
 Lubos Lunak
 l.lu...@suse.cz
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: [Libreoffice] Script to find undocumented classes

2010-12-01 Thread Lubos Lunak
On Tuesday 30 of November 2010, Thorsten Behrens wrote:
> Hi Lubos,
>
> adding documentation for what you describe doesn't help much, if at
> all. Core API is reasonably documented, see offapi/udkapi/sal/basegfx.

 What is core API then? Do things under e.g. libs-core count as well? If yes, 
then just have a look there.

> The most egregious cases of hard-to-grasp methods surely live in the
> applications cores, use lots of weird abbreviations - and to
> document their behaviour succinctly would prolly require as much
> prose as there's code already inside them (i.e. multiple pages).

 ??? Documentation is not about stating again what can be read from the code, 
it is about summarizing and explaining it. A short description of a function 
can save the time reading it all (especially if non-trivial) or help finding 
the needed function, a description of strange code saves the time of figuring 
out why and what it does exactly. Nobody is talking about "int foo = 10; // 
assign 10 to foo". And if a function really needed multiple page of comments 
to explain it, then still I'd rather have that then spend a day (or even 
more, if it really required that much description) figuring it out the moment 
I need to touch it.

> For me, a cleanly designed class with one and only one task (and a
> proper mission statement, maybe - superfluous even for something
> like "String"), does not need much method documentation.
> Suitably-chosen method names, especially if they don't take 10
> parameters, and only work for exactly 42 combinations of them, are
> almost self-documenting.

 Too bad LibO seems to be huge enough to contain many of those that do not fit 
this.

> I simply refuse the notion that adding superficial documentation to
> crappy api gets us anywhere - which does not rule out storing
> hard-won knowledge of class purpose, inner workings, important pre-
> or postconditions at times - and if it makes sense, as method- or
> class-level doxygen documentation.

 I have no idea why you think I'm talking about superficial documentation.

 Besides, it's a question of what is superficial. If you think String is 
trivial, what does str.GetTokenCount('.') give you if str is "a.a." ? I spent 
several minutes on this and would have rather appreciated a one-liner saying 
what the function actually does.

> But to really fix the problem you mention, I'm afraid only
> large-scale refactoring helps.

 Quite possibly. But a) that's a lot more work, b) for that, it still helps to 
know what the code is supposed to do in the first place.

> And wrong documentation surely is worse than no documentation.

 No idea why you think I'm talking about this either.

-- 
 Lubos Lunak
 l.lu...@suse.cz
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: [Libreoffice] Script to find undocumented classes

2010-12-01 Thread Lubos Lunak
On Wednesday 01 of December 2010, Thorsten Behrens wrote:
> see? That's what I meant, documentation for most of the higher-level
> methods is either
>
> a) superficial
> b) so much prose that you're better off debugging the code in the
>first place
>
> (bFull has *a lot* of side effects, and no, I did not bother to
> research all of them for the while)

 See? So nobody actually knows what the code really does, and everytime 
somebody changes something there, they possibly have a slightly different 
understanding of the purpose, resulting in even bigger mess. That's yet 
another advantage of documentation - even you yourself are reminded of what 
the code is supposed to do, and everybody touching such code is less likely 
to run into a different direction.

-- 
 Lubos Lunak
 l.lu...@suse.cz
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: [Libreoffice] Script to find undocumented classes

2010-12-01 Thread Mattias Johnsson
>> Personally I think it would be very nice if source files had just a few
>> lines of comment telling what they are about, from a very high perspective.
>> Every time I am searching for the implementation of some functionality in
>> the OOo/LO codebase, I find myself looking at source files with vaguely
>> correct sounding file names (and vaguely correctly sounding class names
>> inside), but no clue if it actually is what I am looking for.
>>
>> (Yeah, now that we have forked off the codebase, I of course should take up
>> the habit of adding such a few comment lines every time I find out what the
>> code in some particular source file does, especially in cases where one can
>> easily be mislead and spend hours looking at irrelevant code before
>> realizing...)
>
>  I think that would be very helpful.

Totally agree.

Sometimes, after floundering around in the code for a while, I
suddenly come across a comment, and things get *so much* clearer, and
feel I find myself overcome with overwhelming gratitude towards the
mysterious comment writing person.

Even if it's short and cryptic. And in German. It helps that much.
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: [Libreoffice] Script to find undocumented classes

2010-12-01 Thread Kohei Yoshida
On Wed, 2010-12-01 at 01:22 -0700, Tor Lillqvist wrote:
> I am sure it is not hard to find people who think the OOo code APIs
> are elegant, consistent and intuitive. And they would be equally
> right.

Well, I've been working on OOo (and now LibO) code base roughly about 6
years now, and I haven't come across a single person who thinks OOo's
code APIs are elegant, consistent and intuitive.  And I myself is not
one of them. :-P

But I've already become immune to this hard-to-figure-out code base, so
I stopped thinking that way years ago.  I've even trained myself to
ignore those German comments.  When I see functions, I see chunks of
code, not functions. ;-)

Kohei

-- 
Kohei Yoshida, LibreOffice hacker, Calc


___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: [Libreoffice] Script to find undocumented classes

2010-12-01 Thread Kohei Yoshida
On Wed, 2010-12-01 at 13:37 +0100, Lubos Lunak wrote:
> On Wednesday 01 of December 2010, Thorsten Behrens wrote:
> > see? That's what I meant, documentation for most of the higher-level
> > methods is either
> >
> > a) superficial
> > b) so much prose that you're better off debugging the code in the
> >first place
> >
> > (bFull has *a lot* of side effects, and no, I did not bother to
> > research all of them for the while)
> 
>  See? So nobody actually knows what the code really does, and everytime 
> somebody changes something there, they possibly have a slightly different 
> understanding of the purpose, resulting in even bigger mess. That's yet 
> another advantage of documentation - even you yourself are reminded of what 
> the code is supposed to do, and everybody touching such code is less likely 
> to run into a different direction.

I'm with Lubos here, but I believe we are slightly talking past each
other.  What Thorsten is saying (have idutils, ctags etc to power your
editor) is pretty much required to battle this code base efficiently.
But that doesn't take away the fact that the code base is massively
under-documented or have cryptic structure in some places.  And I've
seen many instances of what Lubos is saying (multiple people adding
their own stuff to the same method to bloat it into a kitchen sink that
takes so many conditional bool parameters) happening in many areas of
the code base.  I would like to have some sort of tool to analyze the
cyclomatic complexity of our code base to see if there are any hot spots
in terms of code complexity.

Now, I also agree with Thorsten that not all methods need to be
documented; some methods are quite easy to figure out from their method
names, and others are just methods extracted from other methods as part
of refactoring, whose purpose are still unknown (they were just
identical blocks of code that could be extracted into a common
function).  In the latter case I sometimes don't even know the intent of
such common block of code.

The practice I follow roughly is that, name a method something
descriptive but not to make it too long, and if there are any gotchas or
intent that can not easily be deciphered from the method name or the
content of the code, I try to put that into its comment in the header.

I think the common idiom is that, the code tells you what the method
does, and the comment tells you what the intention of the method is.
So, I personally think it's a good practice to spell out the intention
of a method in its comment if it's not obvious.  But I don't think we
need to comment a method just to re-iterate what it does.

Examples

This is IMO excessive commenting.

/**
 * Translate the passed string into English. 
 * 
 * @param str original string
 * 
 * @return English translation.
 */
string translateToEnglish(const string& str);

This is IMO good commenting.

/**
 * Store new'ed instance of English translation into internal foo 
 *  container. The container manages its life-cycle, so don't delete
 *  stored string instances!
 * 
 * @return true if the string instance is successfully stored, false 
 * if the string was not stored.  The caller must delete the
 * string instance when this method returns false.
 */
bool storeEnglishTranslation(const string* pStr);

Having said all this, I'm a bit concerned that if we start having a
code-comment campaign, and end up having methods with incorrect comments
because only the original authors of the methods know what the intents
of their methods were.  So, I'm not sure if we should go that far,
especially with the existing methods whose authors were not any of us.

Kohei

-- 
Kohei Yoshida, LibreOffice hacker, Calc


___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: [Libreoffice] Script to find undocumented classes

2010-12-01 Thread Michael Meeks

On Wed, 2010-12-01 at 14:25 -0500, Kohei Yoshida wrote:
> But I've already become immune to this hard-to-figure-out code base, so
> I stopped thinking that way years ago.  I've even trained myself to
> ignore those German comments.  When I see functions, I see chunks of
> code, not functions. ;-)

The ability to quickly read, and work effectively on a vast, gnarly,
un-documented code-base is an incredibly valuable one - it turns out
most commercial code-bases are a total mess internally; LibreOffice is
nothing special. Furthermore, the code is -always- correct, and comments
bit-rot at an amazing rate.

However; I'm fairly convinced that there is a happy medium somewhere
closer to Lubos than Thorsten but in-between - I don't think that
investing tons of time into blindly documenting the behaviour of every
method is at all necessary; yet clearly having good, descriptive
per-method (and often more usefully per-class) documentation is really
important. IMHO we cannot and should not put a break on writing that
just to improve merge-ability, so I applaud another of Miklos' great
hacks :-)

Finally - wrt. ultimate code quality - I tend to think that growing our
test suite can also enable the re-factoring that can make our APIs both
pretty and intuitive over time, by providing good, clean, and wide
ranging tests that will catch inadvertent changes in side-effects
as/when they are introduced.

Regards,

Michael.

-- 
 michael.me...@novell.com  <><, Pseudo Engineer, itinerant idiot

___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: [Libreoffice] Script to find undocumented classes

2010-12-01 Thread Thorsten Behrens
Lubos Lunak wrote:
>  See? So nobody actually knows what the code really does, and everytime 
> somebody changes something there, they possibly have a slightly different 
> understanding of the purpose, resulting in even bigger mess. That's yet 
> another advantage of documentation - even you yourself are reminded of what 
> the code is supposed to do, and everybody touching such code is less likely 
> to run into a different direction.
> 
Hi Lubos,

ok, so let me elaborate my (maybe extreme) point a bit.

Documentation is a crutch, a feeble attempt to help us mere humans
to grok big code bases. Good code needs only few comments, because
it speaks for itself - it may need some mission statements on
module- or class level, mostly to get the big picture across. And
that is beneficial on many levels, none the least because, otherwise
the more documentation you have, the higher the likelihood it's
out-of-sync with your code.

Sadly, we don't have good code in most places. So what benefit does
documentation yield? Module- and class-level documentation will
still be a helpful bridge between broader concepts (like "what's an
EditEngine good for", what's the purpose of a "SwFrmFmt"), and
bite-sized pieces of code someone could reasonable look into in
finite time.

But from method-level documentation, I expect a detailed, succinct
description on what the method does, including error conditions,
invariants, pre- and postconditions. Over the top you say? Nope.
Because you'll need to know anyway, to be able to correctly use the
method. 

That's why I'm cautiously avoiding to write detailed documentation
for a function like QuickFormatDoc(bool) - I'll surely miss
something, and even if not, it'll be wrong the day someone adds
another bugfix to the edit engine. You're absolutely right with your
statements above, that method purpose is often fuzzy, even messy -
but that's *already the case* - adding documentation does not make 
it go away!

The thing is, after 15 years of maintenance, most code, especially
high-level one, is the opposite of general-purpose - it's been
tweaked to interoperate with exactly the classes & methods it's
interoperating with today. You need to use it in new contexts? Be
prepared to fix it then. Does documentation help you there? Yes, if
it helps you to understand the broader context quicker. Does
detailed method-level documentation help you? I don't think so, for
application-level code. It has an obscenely bad cost-benefit-ratio -
better go read or debug the code. But you'll want good unit tests,
to make sure your modifications didn't break existing clients (a way
of "documenting" code btw, that's never out-of-sync, and cheaply
verifiable - err, well, ideally ;))

In conclusion, maybe we're somewhat apart conceptually, but when
actually adding documentation, I guess there will be little
differences. I'd especially like to have vcl/tools/svtools classes
reasonably documented - it's a sweet spot between effort, and
benefitting many people at the same time. And you may notice that,
if you find doxygen-style documentation in some gui, gfx, or impress
header files, it's from me. ;)

Cheers,

-- Thorsten


pgpKe6JCLBFPr.pgp
Description: PGP signature
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: [Libreoffice] Script to find undocumented classes

2010-12-01 Thread Pierre-André Jacquod
Beware - I am a newcomer - maybe my point of view is quite wrong due to
lack of understanding. But then at least I will understand something
better :- )

My first remark: stepping in seems to me to be very difficult despite
the very friendly behaviour of people here.

> un-documented code-base is an incredibly valuable one - it turns out
> most commercial code-bases are a total mess internally; LibreOffice is
> nothing special. 

Gnap... depend of which software you are speaking, then... For having
(to) read a lot of code in a big software, I can say that
this one (I mean this commercial one) at least is well documented and
quite well written. And no, I do not work for them:- )

> IMHO we cannot and should not put a break on writing that
> just to improve merge-ability, 
Why not drop merging from OOO after LibO 3.3?  As newcomer, I have read
x times "not here due to merge / not touch due to merge"... Then I am
reluctant to touch some parts...At least, the burden should be on side
of person merging OOo into LibO, not be of a concern for people
contributing to LibO.

> enable the re-factoring that can make our APIs both
> pretty and intuitive over time,
I think would very help any newcomer stepping in. Currently hunting
warning, I do it deliberately in order to get used to the code and uses.
But even there, sometimes I skip the point, not being able after
following the function calling the function calling... and determining
for sure that the change would have a cross effect. And I do not even
tried yet to change a feature.
Deliberate choice from me, clear, but yeah...
Regards


___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: [Libreoffice] Script to find undocumented classes

2010-12-01 Thread Tor Lillqvist
> Why not drop merging from OOO after LibO 3.3? 

Because there is still a significant number of people working on OOo, and they 
are in many cases the best experts there are on the code they are working on, 
and they are doing significant and useful improvements? Because there is no 
licensing reason that would prevent us from being able to take their code?

Note that I am not saying we want to merge *everything* that OOo does. In some 
parts of the code, we might have good reasons to go a separate way.

--tml


___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: [Libreoffice] Script to find undocumented classes

2010-12-02 Thread Lubos Lunak
On Wednesday 01 of December 2010, Thorsten Behrens wrote:
> ok, so let me elaborate my (maybe extreme) point a bit.
>
> Documentation is a crutch, a feeble attempt to help us mere humans
> to grok big code bases. Good code needs only few comments, because
> it speaks for itself - it may need some mission statements on
> module- or class level, mostly to get the big picture across. And
> that is beneficial on many levels, none the least because, otherwise
> the more documentation you have, the higher the likelihood it's
> out-of-sync with your code.

 The first line looks true, but the rest is from a superhuman, because for us 
mere humans documentation does have a value, even if the code would be 
perfect. It's mostly because we happen to be mere humans and do not have the 
brain capacity to remember everything or the time to analyse everything in 
detail. Documentation (and that is not just comments, but also API docs for 
example) is, if nothing else, a time-saver.

 To have a real example, see the sub-thread following 
http://lists.freedesktop.org/archives/libreoffice/2010-December/003756.html . 
Three developers, two of which I think quite highly of (I don't know 
Mattias), in a row failed to use properly a primitive class and ended up 
using a hacked up solution, because the moron that created this class saved 
at most few minutes by being too lazy to add at least "use 
SwCursor::RestoreSavePos()", eventually leading to much more time wasted and 
suboptimal code.

 A frustration-saver too, now that I think of it. I certainly had one of those 
WTF moments when trying to figure the problem out.

> Sadly, we don't have good code in most places. So what benefit does
> documentation yield? Module- and class-level documentation will
> still be a helpful bridge between broader concepts (like "what's an
> EditEngine good for", what's the purpose of a "SwFrmFmt"), and
> bite-sized pieces of code someone could reasonable look into in
> finite time.

 Have you actually ever worked with well-documented codebase? I can assure you 
that I've spent much more time reading API documentation of KDE library code 
(and saving me ever more time figuring it out myself) than I have spent 
writing it for my KDE library code (which is a non-trivial amount).

> But from method-level documentation, I expect a detailed, succinct
> description on what the method does, including error conditions,
> invariants, pre- and postconditions. Over the top you say? Nope.
> Because you'll need to know anyway, to be able to correctly use the
> method.

 I see. Yes, your point maybe is extreme, because to me it looks like you're 
saying that documentation should be perfect and nothing less should be 
accepted, so if it can't be perfect, it has to be none, which will keep 
people clueless, so they at least won't touch the code.

> That's why I'm cautiously avoiding to write detailed documentation
> for a function like QuickFormatDoc(bool) - I'll surely miss
> something, and even if not, it'll be wrong the day someone adds
> another bugfix to the edit engine. You're absolutely right with your
> statements above, that method purpose is often fuzzy, even messy -
> but that's *already the case* - adding documentation does not make
> it go away!

 Sorry, but you really do not understand what I am actually talking about. I'm 
not asking for anything ridiculous like having a paragraph explaining every 
line, I'm saying that everything non-trivial should have at least some 
documentation (or that should be at least goal, I don't expect everybody to 
start documenting everything today and dropping all other work). It is no big 
deal to require this for new code and whenever somebody runs into a problem 
or just feels like writing it down. KDE libraries are required to have this 
for all public API and nobody has a problem with that, and, just as well, it 
would be pretty unusual for 3 developers to fail using a class with just a 
ctor and a dtor.

 As for documentation getting out of date, especially when it's API 
description or code comments, I generally consider those getting obsolete 
being because the developer causing them was a moron who either broke public 
API or just quickly hacked up something without thinking about it.

 And even if that happens, I disagree with obsolete docs being worse than no 
docs - even wrong docs can help if you have a problem, you can search the 
history or similar, while without docs you're simply on your own and that's 
it.

 You know, if, after figuring out all these things about QuickFormatDoc(bool) 
and, you at least wrote down "the bool argument probably means that 
 but it has a lot of side-effects", you would very likely still 
have helped Sebastian a lot. Even that counts, nobody wants a load of prose 
from you.

> In conclusion, maybe we're somewhat apart conceptually, but when
> actually adding documentation, I guess there will be little
> differences. I'd especially like to have vcl/tools/svtools classes

Re: [Libreoffice] Script to find undocumented classes

2010-12-02 Thread Thorsten Behrens
Hi Lubos,

you wrote:
> I'm not asking for anything ridiculous like having a paragraph
> explaining every line, I'm saying that everything non-trivial
> should have at least some documentation
>
Good, then we're mostly on the same page. Then we're both talking
about mission statements, higher-level interrelations etc.

> You know, if, after figuring out all these things about
> QuickFormatDoc(bool) and, you at least wrote down "the bool
> argument probably means that  but it has a lot of
> side-effects", you would very likely still have helped Sebastian a
> lot.
> 
Well, I guess that's the point of contention we have here. A comment
like that is useless in my book.

And I somehow wanted to preemptively counter the notion that, like a
cargo-cult, putting documentation on LibO code will magically make
it intuitively usable - just because KDE is intuitively programmable, 
and has documentation.

But I guess this topic is now thoroughly discussed to death, I hope
my point, and my preference, is clear. We'd all love to have more 
documentation inside LibO, but neither me, nor anybody else I 
believe, will start mandating certain ways of documenting. We
welcome cleanups, fixes and features - and if that comes with
helpful header documentation, extra brownie points. ;)

So just in case, let's agree to disagree & keep the patches coming!
:)

Cheers,

-- Thorsten


pgpRI3Gn8CfOF.pgp
Description: PGP signature
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: [Libreoffice] Script to find undocumented classes

2010-12-03 Thread Jan Holesovsky
Hi all,

On 2010-12-02 at 21:59 +0100, Thorsten Behrens wrote:

> So just in case, let's agree to disagree & keep the patches coming!
> :)

As a conclusion, what about to combine Miklos' check for the missing
documentation with a commit hook, so that it does not allow you to
commit _new_ files without (at least the high level) documentation? ;-)

Miklos - please, would that be doable without big impact on the speed of
'git commit'?

Regards,
Kendy

___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: [Libreoffice] Script to find undocumented classes

2010-12-03 Thread Thorsten Behrens
Jan Holesovsky wrote:
> As a conclusion, what about to combine Miklos' check for the missing
> documentation with a commit hook, so that it does not allow you to
> commit _new_ files without (at least the high level) documentation? ;-)
> 
Hi Kendy,

I find it surprising me actually saying this, but - for the while, I
think this would be crossing the line of solving a social problem
by technical means. ;)

Cheers,

-- Thorsten


pgpHUGF9J5UDV.pgp
Description: PGP signature
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: [Libreoffice] Script to find undocumented classes

2010-12-03 Thread Miklos Vajna
On Fri, Dec 03, 2010 at 11:14:54AM +0100, Thorsten Behrens 
 wrote:
> I find it surprising me actually saying this, but - for the while, I
> think this would be crossing the line of solving a social problem
> by technical means. ;)

Additionally I'm not aware of a method to tell doxygen to check just a
part of a file. And issuing a warning about 'class Foo is not
documented' just because someone touched class Bar in the same file is
indeed incorrect. ;)


pgpgPNVYYxFDF.pgp
Description: PGP signature
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: [Libreoffice] Script to find undocumented classes

2010-12-03 Thread Wols Lists
On 03/12/10 12:11, Miklos Vajna wrote:
> On Fri, Dec 03, 2010 at 11:14:54AM +0100, Thorsten Behrens 
>  wrote:
>> I find it surprising me actually saying this, but - for the while, I
>> think this would be crossing the line of solving a social problem
>> by technical means. ;)
> Additionally I'm not aware of a method to tell doxygen to check just a
> part of a file. And issuing a warning about 'class Foo is not
> documented' just because someone touched class Bar in the same file is
> indeed incorrect. ;)
>
I keep seeing all this stuff about Doxygen :-) I'm a bit of a newbie at
this :-) so might it not be a good idea, on the wiki under development,
to put a very basic page about doxygen with pointers to more detailed stuff?

That way, people like me have a handle to get started. I know I can
google for it, but a very basic "in libreoffice we do this" guide in the
wiki would be a big help.

Cheers,
Wol
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: [Libreoffice] Script to find undocumented classes

2010-12-03 Thread Jan Holesovsky
Hi Miklos, Thorsten,

On 2010-12-03 at 13:11 +0100, Miklos Vajna wrote:

> > I find it surprising me actually saying this, but - for the while, I
> > think this would be crossing the line of solving a social problem
> > by technical means. ;)
> 
> Additionally I'm not aware of a method to tell doxygen to check just a
> part of a file. And issuing a warning about 'class Foo is not
> documented' just because someone touched class Bar in the same file is
> indeed incorrect. ;)

That's why I highlighted that this would be done only with _new_ files,
ie. the files that have have been git add'ed, and did not exist in the
repository before.  And we can further limit that only to .hxx/.h.

As to the crossing the line - the first time it won't let you commit,
and you'll be angry, the second time it won't let you commit, and you'll
just fix that, and the third time you'll comment just naturally, and
won't even hit the check :-)  This worked with the warnings about how to
structure the git commit logs [do you ever hit the hook that the 2nd
line in the commit log should be empty?], so I don't see why it
shouldn't work in this case ;-)

Regards,
Kendy

___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: [Libreoffice] Script to find undocumented classes

2010-12-03 Thread Jan Holesovsky
Hi Wol,

On 2010-12-03 at 13:43 +, Wols Lists wrote:

> >> I find it surprising me actually saying this, but - for the while, I
> >> think this would be crossing the line of solving a social problem
> >> by technical means. ;)
> > Additionally I'm not aware of a method to tell doxygen to check just a
> > part of a file. And issuing a warning about 'class Foo is not
> > documented' just because someone touched class Bar in the same file is
> > indeed incorrect. ;)
> >
> I keep seeing all this stuff about Doxygen :-) I'm a bit of a newbie at
> this :-) so might it not be a good idea, on the wiki under development,
> to put a very basic page about doxygen with pointers to more detailed stuff?

The hook can directly point to a page how to do it right ;-)  Basically,
instead of doing

class Foo {
int ugh;
};

you'd do:

/**
This class implements access to your low level Foo machinery.

It is to be used by This and That.  [and probably some description of
the pitfalls, if any]
*/
class Foo {
int ugh;
};

and you'd be done, the hook would be happy.  Of course, special bonus
would be for

/// File descriptor of the Foo machinery.
int ugh;

Or did we actually not reached even this level of agreement? ;-)

Regards,
Kendy

___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: [Libreoffice] Script to find undocumented classes

2010-12-03 Thread Miklos Vajna
On Fri, Dec 03, 2010 at 03:04:55PM +0100, Jan Holesovsky  wrote:
> That's why I highlighted that this would be done only with _new_ files,
> ie. the files that have have been git add'ed, and did not exist in the
> repository before.  And we can further limit that only to .hxx/.h.

Ah, that makes a difference.

> As to the crossing the line - the first time it won't let you commit,
> and you'll be angry, the second time it won't let you commit, and you'll
> just fix that, and the third time you'll comment just naturally, and
> won't even hit the check :-)  This worked with the warnings about how to
> structure the git commit logs [do you ever hit the hook that the 2nd
> line in the commit log should be empty?], so I don't see why it
> shouldn't work in this case ;-)

I just added support for checking given files only (relative to the
current directory) instead of the old "check all files in the current
directory and subdirectories" behaviour. Should I write the git hook
part of it, or can you do it?

The only nontrivial part is how to to reach the scratch folder from the
bootstrap repo. :)


pgpsUPSCKnsqS.pgp
Description: PGP signature
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: [Libreoffice] Script to find undocumented classes

2010-12-03 Thread Jan Holesovsky
Hi Miklos,

On 2010-12-03 at 16:19 +0100, Miklos Vajna wrote:

> > That's why I highlighted that this would be done only with _new_ files,
> > ie. the files that have have been git add'ed, and did not exist in the
> > repository before.  And we can further limit that only to .hxx/.h.
> 
> Ah, that makes a difference.

Good :-)

> > As to the crossing the line - the first time it won't let you commit,
> > and you'll be angry, the second time it won't let you commit, and you'll
> > just fix that, and the third time you'll comment just naturally, and
> > won't even hit the check :-)  This worked with the warnings about how to
> > structure the git commit logs [do you ever hit the hook that the 2nd
> > line in the commit log should be empty?], so I don't see why it
> > shouldn't work in this case ;-)
> 
> I just added support for checking given files only (relative to the
> current directory) instead of the old "check all files in the current
> directory and subdirectories" behaviour. Should I write the git hook
> part of it, or can you do it?

If you can do, that would be great.

> The only nontrivial part is how to to reach the scratch folder from the
> bootstrap repo. :)

I'd say, just move it to bootstrap/bin, and enable in the
bootsrap/git-hooks only?

Thank you a lot,
Kendy

___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: [Libreoffice] Script to find undocumented classes

2010-12-03 Thread Kohei Yoshida
On Fri, 2010-12-03 at 10:23 +0100, Jan Holesovsky wrote:
> Hi all,
> 
> On 2010-12-02 at 21:59 +0100, Thorsten Behrens wrote:
> 
> > So just in case, let's agree to disagree & keep the patches coming!
> > :)
> 
> As a conclusion, what about to combine Miklos' check for the missing
> documentation with a commit hook, so that it does not allow you to
> commit _new_ files without (at least the high level) documentation? ;-)

I'm actually NOT in favor of this.  As much as I believe in providing
good code documentation for new code, this is a bit too far.

Kohei

-- 
Kohei Yoshida, LibreOffice hacker, Calc


___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: [Libreoffice] Script to find undocumented classes

2010-12-03 Thread Kohei Yoshida
On Fri, 2010-12-03 at 11:08 -0500, Kohei Yoshida wrote:
> On Fri, 2010-12-03 at 10:23 +0100, Jan Holesovsky wrote:
> > Hi all,
> > 
> > On 2010-12-02 at 21:59 +0100, Thorsten Behrens wrote:
> > 
> > > So just in case, let's agree to disagree & keep the patches coming!
> > > :)
> > 
> > As a conclusion, what about to combine Miklos' check for the missing
> > documentation with a commit hook, so that it does not allow you to
> > commit _new_ files without (at least the high level) documentation? ;-)
> 
> I'm actually NOT in favor of this.  As much as I believe in providing
> good code documentation for new code, this is a bit too far.

My rationale: Many times when I work on feature branches, I commit stuff
but intentionally not provide documentation because the role of the
class/method/whatever may change during the course of the
implementation.  This requirement would break my workflow, and I
wouldn't appreciate that.

Encouraging good documentation is a must, but making it a requirement
even for new files unconditionally is bad.

Kohei

-- 
Kohei Yoshida, LibreOffice hacker, Calc


___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: [Libreoffice] Script to find undocumented classes

2010-12-03 Thread Miklos Vajna
On Fri, Dec 03, 2010 at 11:14:12AM -0500, Kohei Yoshida  
wrote:
> My rationale: Many times when I work on feature branches, I commit stuff
> but intentionally not provide documentation because the role of the
> class/method/whatever may change during the course of the
> implementation.  This requirement would break my workflow, and I
> wouldn't appreciate that.
> 
> Encouraging good documentation is a must, but making it a requirement
> even for new files unconditionally is bad.

Hm, what about enforcing it only in master / libreoffice-* branches?
(Just an idea.)


pgpfhUAnKQeGx.pgp
Description: PGP signature
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: [Libreoffice] Script to find undocumented classes

2010-12-03 Thread Kohei Yoshida
On Fri, 2010-12-03 at 17:34 +0100, Miklos Vajna wrote:
> On Fri, Dec 03, 2010 at 11:14:12AM -0500, Kohei Yoshida  
> wrote:
> > My rationale: Many times when I work on feature branches, I commit stuff
> > but intentionally not provide documentation because the role of the
> > class/method/whatever may change during the course of the
> > implementation.  This requirement would break my workflow, and I
> > wouldn't appreciate that.
> > 
> > Encouraging good documentation is a must, but making it a requirement
> > even for new files unconditionally is bad.
> 
> Hm, what about enforcing it only in master / libreoffice-* branches?
> (Just an idea.)

That would be fine with me.  I would still like to avoid making it a
requirement, but keeping it master only is bearable.

I still prefer not having that as a requirement though.  Running the
missing-doc script every now and then like we do with cppchecks and
Caolan's callcatcher would be my preferred approach.

Kohei

-- 
Kohei Yoshida, LibreOffice hacker, Calc


___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: [Libreoffice] Script to find undocumented classes

2010-12-03 Thread Thorsten Behrens
Jan Holesovsky wrote:
> As to the crossing the line - the first time it won't let you commit,
> and you'll be angry, the second time it won't let you commit, and you'll
> just fix that, and the third time you'll comment just naturally, and
> won't even hit the check :-)  This worked with the warnings about how to
> structure the git commit logs [do you ever hit the hook that the 2nd
> line in the commit log should be empty?], so I don't see why it
> shouldn't work in this case ;-)
> 
Hi Kendy,

I was actually serious with my comment. The "2nd line must be blank"
is a triviality, and a no-brainer. Banning tabs in code was at times
actually work, and I've seen commits where people circumvented the
hook. Forcing people to do what they wouldn't do otherwise is not
the best strategy to get the best results...

Cheers,

-- Thorsten


pgpbryZupxi7s.pgp
Description: PGP signature
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice