Hacking on shill

To keep the shill source code consistent, please follow the conventions below:

  • Follow the Chromium C++ style guide

    If you use Emacs, the Google C Style mode will help you with the formatting aspects of style. (Chromium Style generally follows Google Style). Get the Emacs mode at https://github.com/google/styleguide/blob/gh-pages/google-c-style.el

  • When deferring work from a signal handler (e.g. a D-Bus callback) to the event loop, name the deferred work function by adding Task to the name of the function deferring the work. E.g.

    void Modem::Init() {
      dispatcher_->PostTask(FROM_HERE,
                            task_factory_.NewRunnableMethod(&Modem::InitTask));
    }
    

    RATIONALE: The naming convention makes the relationship between the signal handler and the task function obvious, at-a-glance.

  • When adding verbose log messages for debug purposes, use the SLOG marco and its variants (see logging.h for details).

    • Choose the appropriate scope and verbose level for log messages. E.g.

      SLOG(WiFi, 1) << message;  // for WiFi related code
      
    • Before defining a new scope, check if any existing scope defined in scope_logger.h already fulfills the needs.

    • To add a new scope:

      1. Add a new value to the Scope enumerated type in scope_logger.h. Keep the values sorted as instructed in the header file.
      2. Add the corresponding scope name to the kScopeNames array in scope_logger.cc.
      3. Update the GetAllScopeNames test in scope_logger_test.cc.
  • When adding externally visible (i.e. via RPC) properties to an object, make sure that a) its setter emits any change notification required by Chrome, and that b) its setter properly handles no-op changes.

    Test that the property changes are handled correctly by adding test cases similar to those in CellularServiceTest.PropertyChanges, and CellularServiceTest.CustomSetterNoopChange.

  • When performing trivial iteration through a container, prefer using range based for loops, preferably:

    for (const auto& element : container) {
    

    Remove const where necessary if the element will be modified during the loop. Removal of the const and reference for trivial types is allowed but not necessary.