Difference between revisions of "Janitorial tasks"

From Inkscape Wiki
Jump to navigation Jump to search
(Adding example)
Line 1: Line 1:
 
This is a quick list of tasks needing to be done, that no one has gotten around to yet.  They're not in any order yet, and simply have a note at the end with who added them to the list, so if you have questions, you can ask them.  (Or, ask the devel mailing list.)
 
This is a quick list of tasks needing to be done, that no one has gotten around to yet.  They're not in any order yet, and simply have a note at the end with who added them to the list, so if you have questions, you can ask them.  (Or, ask the devel mailing list.)
  
 +
== Inline Function Removal ==
 +
* In src/xml/repr.h, there are a bunch of inline functions that are trivial wrappers for SPRepr member functions.  Your task, should you choose to accept it:  replace all calls to those wrapper functions with direct calls to the wrapped member functions, then remove the unused wrapper functions. (mental) (patches have been applied from Robert Crosbie -- what's left for this item?)
  
* In src/xml/repr.h, there are a bunch of inline functions that are trivial wrappers for SPRepr member functions.  Your task, should you choose to accept it:  replace all calls to those wrapper functions with direct calls to the wrapped member functions, then remove the unused wrapper functions. (mental) (patches have been applied from Robert Crosbie -- what's left for this item?)
 
  
 +
== Class Design Consistency ==
 
* Adding default, private, un-defined copy ctor and operator = . As a rule, all C++ classes should have an explicit copy constructor and assignment operator (operator =). Instead of defaulting to the standard one, any classes that don't already have them should have the two declared in the "private:" section of the class, but never actually have those functions defined. (JonCruz)
 
* Adding default, private, un-defined copy ctor and operator = . As a rule, all C++ classes should have an explicit copy constructor and assignment operator (operator =). Instead of defaulting to the standard one, any classes that don't already have them should have the two declared in the "private:" section of the class, but never actually have those functions defined. (JonCruz)
 
<pre>
 
<pre>
Line 14: Line 16:
 
</pre>
 
</pre>
  
 +
 +
== Add Assertions ==
 +
* Look for functions that are using pointer variables without checking if they're NULL first.  Add a call to g_assert, g_return_if_fail, etc.:
 +
 +
void inkscape_dispose(GObject * obj) {
 +
    Inkscape::Application *inkscape = (Inkscape::Application *) object;
 +
    g_assert(inkscape != NULL);  // <-- add this assertion
 +
 +
    while (inkscape->documents) {
 +
        /* ... */
 +
    }
 +
}
 +
 +
== Improving Headers for Compilation Speed ==
 +
When a header file changes, any file that includes it is recompiled.  This becomes a problem when a given header file is included by other header files, as it leads to a vast header include tree, wherein a small change to a seemingly minor header file causes massive rebuilds of much of the codebase. 
 +
 +
Fortunately for us, there are tricks to fighting this!  Here's what to do:
 +
 +
* Find a header that includes several other headers.  For example, the header for might have a class Bar with a member of type Foo*, thus it includes "foo.h" to get the definition of that type.  You can prune this by using a 'forward declaration':
 +
 +
//#include "foo.h"  /* <-- kill the header include! */
 +
class Foo;          /* <-- replace with a forward declaration */
 +
class Bar {
 +
    Foo* _foo;
 +
};
 +
 +
* Move class documentation from the .h to the .cpp.  We have an automated code documentation generator called Doxygen, that generates HTML docs from comments in the .h or .cpp files.  Sometimes people put the code docs in the .h, but this means that whenever you update the docs, everything including that .h has to be recompiled, even if there are no _real_ changes to the code.  Instead, move all these comments into the corresponding .cpp file.  It's okay to have a short paragraph comment at the top of the .h file explaining what it is.
 +
 +
 +
== Syntactical Cleanup ==
 
* Removing naughty whitespace (tabs, trailing whitespace). Tabs in the source just lead to many troubles, so they aren't supposed to be used. One problem is that just fixing them to spaces does introduce extra diffs in the CVS history, so someone wanting to remove tabs should first figure out where they are and then talk over an approach for cleansing them. Trailing whitespace is also a non-visible but diff-confusing issue, and should also eventually tracked down and removed. (JonCruz)
 
* Removing naughty whitespace (tabs, trailing whitespace). Tabs in the source just lead to many troubles, so they aren't supposed to be used. One problem is that just fixing them to spaces does introduce extra diffs in the CVS history, so someone wanting to remove tabs should first figure out where they are and then talk over an approach for cleansing them. Trailing whitespace is also a non-visible but diff-confusing issue, and should also eventually tracked down and removed. (JonCruz)
  
 +
* Make sure all files include the standard copyright/license info
 +
 +
* Make sure all files include the emacs and vim style variables at end
 +
 +
 +
== Documentation ==
 +
* Go through .h files and put a sentence or two comment at the top explaining what the class is.  Don't be too detailed; details belong in the comments in the corresponding .cpp file.
 +
 +
* Add comments to each function in .cpp files.  Pick a .cpp file and read through it.  Before each function, add comments describing what the function does.  See the files in the inkscape/extensions/ directory as examples.  We want ALL of the Inkscape sourcecode documented like that.
  
'''more details needed'''
+
== More details needed ==
 
* Marking C-Style casts (JonCruz)
 
* Marking C-Style casts (JonCruz)
 
* Fixing C-Style casts (JonCruz)
 
* Fixing C-Style casts (JonCruz)
  
 
* Fix all the gcc 3.4.2 warnings (inkblotter)
 
* Fix all the gcc 3.4.2 warnings (inkblotter)

Revision as of 18:59, 13 February 2005

This is a quick list of tasks needing to be done, that no one has gotten around to yet. They're not in any order yet, and simply have a note at the end with who added them to the list, so if you have questions, you can ask them. (Or, ask the devel mailing list.)

Inline Function Removal

  • In src/xml/repr.h, there are a bunch of inline functions that are trivial wrappers for SPRepr member functions. Your task, should you choose to accept it: replace all calls to those wrapper functions with direct calls to the wrapped member functions, then remove the unused wrapper functions. (mental) (patches have been applied from Robert Crosbie -- what's left for this item?)


Class Design Consistency

  • Adding default, private, un-defined copy ctor and operator = . As a rule, all C++ classes should have an explicit copy constructor and assignment operator (operator =). Instead of defaulting to the standard one, any classes that don't already have them should have the two declared in the "private:" section of the class, but never actually have those functions defined. (JonCruz)
class Foo {
   ...
private:
    Foo& operator= ( const Foo& other );
    Foo( const Foo& other );
};


Add Assertions

  • Look for functions that are using pointer variables without checking if they're NULL first. Add a call to g_assert, g_return_if_fail, etc.:
void inkscape_dispose(GObject * obj) {
    Inkscape::Application *inkscape = (Inkscape::Application *) object;
    g_assert(inkscape != NULL);   // <-- add this assertion
    while (inkscape->documents) {
       /* ... */
   }
}

Improving Headers for Compilation Speed

When a header file changes, any file that includes it is recompiled. This becomes a problem when a given header file is included by other header files, as it leads to a vast header include tree, wherein a small change to a seemingly minor header file causes massive rebuilds of much of the codebase.

Fortunately for us, there are tricks to fighting this! Here's what to do:

  • Find a header that includes several other headers. For example, the header for might have a class Bar with a member of type Foo*, thus it includes "foo.h" to get the definition of that type. You can prune this by using a 'forward declaration':
//#include "foo.h"  /* <-- kill the header include! */
class Foo;          /* <-- replace with a forward declaration */
class Bar {
    Foo* _foo;
};
  • Move class documentation from the .h to the .cpp. We have an automated code documentation generator called Doxygen, that generates HTML docs from comments in the .h or .cpp files. Sometimes people put the code docs in the .h, but this means that whenever you update the docs, everything including that .h has to be recompiled, even if there are no _real_ changes to the code. Instead, move all these comments into the corresponding .cpp file. It's okay to have a short paragraph comment at the top of the .h file explaining what it is.


Syntactical Cleanup

  • Removing naughty whitespace (tabs, trailing whitespace). Tabs in the source just lead to many troubles, so they aren't supposed to be used. One problem is that just fixing them to spaces does introduce extra diffs in the CVS history, so someone wanting to remove tabs should first figure out where they are and then talk over an approach for cleansing them. Trailing whitespace is also a non-visible but diff-confusing issue, and should also eventually tracked down and removed. (JonCruz)
  • Make sure all files include the standard copyright/license info
  • Make sure all files include the emacs and vim style variables at end


Documentation

  • Go through .h files and put a sentence or two comment at the top explaining what the class is. Don't be too detailed; details belong in the comments in the corresponding .cpp file.
  • Add comments to each function in .cpp files. Pick a .cpp file and read through it. Before each function, add comments describing what the function does. See the files in the inkscape/extensions/ directory as examples. We want ALL of the Inkscape sourcecode documented like that.

More details needed

  • Marking C-Style casts (JonCruz)
  • Fixing C-Style casts (JonCruz)
  • Fix all the gcc 3.4.2 warnings (inkblotter)