On Tue, Aug 11, 2009 at 8:29 PM, dan nessettdnessett@yahoo.com wrote:
--- On Tue, 8/11/09, Chad innocentkiller@gmail.com wrote:
Neither of these need to be tested directly. If
AutoLoader breaks,
then some other class won't load, and the tests for
that class will
fail. If wfRunHooks() fails, then some hook won't
work, and any test
of that hook will fail.
I will pass on commenting about these for the moment because:
I think what's needed for decent test usage for
MediaWiki is:
- Some test suite is picked. PHPUnit is probably
fine, if it runs
out of the box and doesn't need some extra module to
be installed.
- The test suite is integrated into CodeReview with
nag e-mails for
broken tests.
- A moderate number of tests are written for the test
suite.
Existing parser tests could be autoconverted,
possibly. Maybe someone
paid could be assigned to spend a day or two on this.
- A new policy requires that everyone write tests for
all their bug
fixes and enhancements. Commits that don't add
enough tests will be
flagged as fixme, and reverted if not fixed.
(4) is critical here.
All good stuff, especially (4) - applause :-D.
While we're at it, it would be nice if we instituted
some other
iron-clad policies. Here's a proposal:
- All new functions (including private helper
functions, functions in
JavaScript includes, whatever) must have
function-level documentation
that explains the purpose of the function and
describes its
parameters. The documentation must be enough that no
MediaWiki
developer should ever have to read the function's
source code to be
able to use it correctly. Exception: if a method is
overridden which
is already documented in the base class, it doesn't
need to be
documented again in the derived class, since the
semantics should be
the same.
- All new classes must have high-level documentation
that explains
their purpose and structure, and how you should use
them. The
documentation must be sufficient that any MediaWiki
developer could
understand why they might want to use the class in
another file, and
how they could do so, without reading any of the
source code. Of
course, developers would still have to read the
function documentation
to learn how to use specific functions. There are no
exceptions, but
a derived class might only need very brief
documentation.
- All new config variables must have documentation
explaining what
they do in terms understandable to end-users. They
must describe what
values are accepted, and if the values are complicated
(like
associative arrays), must provide at least one example
that can be
copy-pasted. There are no exceptions.
- If any code is changed so as to make a comment
incorrect, the
comment must be updated to match. There are no
exceptions.
Or whatever. We have *way* too few high-level
comments in our code.
We have entire files -- added quite recently, mind
you, by established
developers -- that have no or almost no documentation
on either the
class or function level. We can really do better
than this! If we
had a clear list of requirements for comments in new
code, we could
start fixme'ing commits that don't have adequate
comments. I think
that would be enough to get people to add sufficient
comments, for the
most part. Without clear rules, though, backed up by
the threat of
reverting, I don't think the situation will improve
here.
Wonderful stuff - more applause.
(Wow, this kind of turned into a thread hijack. :D)
Who cares. It needs to be said.
On the whole "new code" front. Can we all /please/ remember that we're writing PHP5 here. Visibility on all new functions and variables should also be a must.
OK, I must admit I didn't understand that, probably because I'm new to PHP. Can you make this more explicit?
Dan
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
This is slightly OT and I don't want to hijack the main point (getting our shit together for testing), but...
PHP4 did not have proper object oriented support. There was no concept of visibility--all member variables and functions were public. PHP5 fixed this by adding the familiar public/protected/private structure, as well as a bunch of other things that a proper OO language should be able to do.
My point is that not declaring visibility on new code (ie: leaving everything public) is poor form. It usually doesn't take very long to make the decision about what scope to put a function or variable in, and it can always be changed later if the choice was wrong.
-Chad