wiki:ReviewerChecklist

This checklist is meant at GDAL committers that review a contribution (e.g. a new GDAL or OGR driver) before commiting it. Contributors are invited to read it as well, to make sure they have taken the necessary steps to make their contribution more easily integrated.

Reminder: the gist of the work is supposed to be done by the contributor, not by the reviewer. A contributor not willing to fix his work and take into account reviewer's remarks will not probably be a long-term asset for the project.

Code review checks

  • All source files (.h, .c, .cpp, .py, etc...) should have a header with copyright attribution and the text of the GDAL X/MIT license.
  • The contributor should have the right to contribute his work. This can be done by requesting him to send a public email to gdal-dev mailing list mentionning he has right to contribute his work under the X/MIT license.
  • Pay attention if the driver depends on a third-party library. If it is the case, the compilation of the driver must be made conditional to the presence of the library. Drivers should try to re-use existing third-party libraries dependencies as much as possible, e.g. Expat for SAX XML parsing.
  • The patch should be against trunk
  • For a OGR driver, check that the Open() method of the driver (often delegated to a Open() method of the datasource) is selective enough (i.e. it will not accept data files that are not meant for the driver), and robust enough (it will not crash for small variations w.r.t content that it would recognize). Check that it can deal with unusual filenames. For a GDAL driver, similar checks, as well for the optional Identify() method
  • Functions and methods should have a selective enough namespace ("GDAL" or "OGR" prefix, or use of C++ namespace) to avoid symbol collision.
  • Developer guidelines defined in RFC8 should be followed.
  • Check that the code is portable enough :
    • at the time of writing, no C++11 features.
    • independant of host endianness. Use of CPL macros to do byte-swapping
    • be careful about the use of "long" datatype, especially when reading/writing serialized data. It is 32-bit wide on gcc/clang 32bit and MSVC 32/64bit, but 64-bit wide on gcc/clang 64bit.
  • Ideally, the code should use CPL infrastructure when available, for example VSI*L API for file I/O
  • Check that the indentation is consistant (recommended: 4 spaces for consistency with the global code base), and particularly it has no tabulation characters
  • End-of-line characters should be the Unix way (\n)
  • Check that the contribution includes Unix and Windows makefiles
  • Check that the contribution includes documentation (frmt_XXXX.html, drv_XXXX.html as well as links from the main format pages frmts/formats_list.html or ogr/ogrsf_frtms/ogr_formats.html). Documentation should, at minimum, say some words about the format handled by the driver, and, when relevant, describe the particular syntax for the connection string, creation options, configuration options, link to format description, mention needed third-party libraries.
  • Ideally, the contribution should include a Python script to add to the autotest suite, and one or small samples (each < 20 KB ) appropriate to go to autotest/gdrivers/data or autotest/ogr/data
  • If the driver depends on an optional third-party library, the autotest script should gracefully skip the tests when the driver is not available at run-time.

Compile-time checks

  • The code should be as compilation warning-free as possible.
  • You can directly test Linux and Windows compilation with the GDAL Vagrant VM

Run-time checks

  • For a OGR driver, compile the test_ogrsf utility (cd apps; make test_ogrsf) and run sample files with it.
  • Run the provided autotest scripts, natively (and if you can cope with the noise due to Python-related false-positive warnings under Valgrind to detect memory use errors and leaks)
  • Run the utilities (gdalinfo, gdal_translate, ogr2ogr) under Valgrind.

Post-commit checks

  • Once committed, trunk is compiled on Linux, Mac OS X and cross-compiled mingw 32 and 64 bit Travis-CI instances, so you check that the new code does not break those environments.
  • Coverity Scan static analyis is run weekly on Tuesday, 22:00 MDT. Developers/maintainers can request access on the GDAL Project page.
Last modified 2 years ago Last modified on Aug 10, 2015 8:13:06 AM