Contributing

This page explains the contribution practices we follow. Please feel free to reach out to us on Slack at #buildbox to say hi or ask questions!

Raising tickets

  • For bugs:

    • Check the GitLab board in case the bug has already been raised.

    • Raise a ticket following the template, label with “bug”.

    • We will always look into bugs found by users, but depending on their priority we may not be able to commit to fixing them right away. We welcome patch submissions for bugfixes!

  • For feature additions:

    • Check the GitLab board in case the feature has already been flagged up.

    • For significant new features, send a proposal to the mailing list for discussion.

      • Once the approach is agreed upon, raise a ticket in the board which summarises the approach and tasks.

    • For smaller features, raise a ticket in the board and label it appropriately, discussion can take place on the ticket in these cases.

    • As above, we welcome patch submissions for new features!

  • If you plan to work on a ticket, assign it to yourself and pull it from the Open column into:

    • Todo if you’re going to pick it up in the next week, but haven’t started it yet.

    • Doing when you start working on it.

    • Review when you have an MR / MRs attached to the issue that need approval.

    • Blocked when you can’t make any more progress, please also include an explanation of the blocker in the ticket and raise it in the #buildbox Slack channel.

    • Done when the MR is merged - GitLab can do this automatically with a line in the MR to say “When merged, this resolves <issue #>”.

Patch submissions

  • Patches must be submitted as merge requests, and by default we require 2 approvals aside from the committer before merging.

    • Ideally, approval will come from at least one person who is already familiar with the area of the codebase affected. If you’re unsure who to ask, ping us on Slack.

  • Merge requests should have a corresponding issue that they are raised from, or at least one that is linked to within the MR description. For small features, e.g. documentation changes or really small code fixes/cleanup, submitting MRs without a linked issue is fine.

    • MRs with a corresponding issue should have the issue number in the commit message.

  • Please follow the template when raising MRs.

  • Keep MRs small for ease of review - each item that is being addressed should have a separate commit. Unit tests should be a separate commit, as should documentation changes.

  • MRs not yet ready to be merged should be prefixed with WIP:. This is useful for early feedback on branches, and in general for keeping our work transparent.

  • For guidelines on good commit messages, see this blog post.

Commit access

BuildBox has a group of people with commit access currently. The list of committers can be found here. To obtain commit access, someone would need to have submitted a few non-trivial patches to BuildBox, and otherwise have shown good practices with regard to making small, clean commits, giving thorough review, etc. One of the current committers would then propose that person be given commit access, with the decision to be taken by the committers. A person can request that they be granted commit access at this point also.

Coding style

Formatting

C++ sources must be formatted using clang-format 6.0. Each project has a .clang-format file in the root directory of its repo (the format is shared across all BuildBox projects).

After making changes to files, they can be formatted by running git-clang-format-6.0 -f --extensions cpp,h from the project’s root directory.

The format is enforced by the check_formatting job of the CI, and commits must pass in order to be acceptable for merging.

Logging

In order to assist with diagnosing problems and aid with debugging, we aim to have descriptive log messages that at the same time do not become overwhelming.

buildbox-common provides a logging facility that is used in all BuildBox projects and it provides different levels:

  • BUILDBOX_LOG_INFO: used for messages that give feedback to the user that progress is being made or options were acknowledged.

  • BUILDBOX_LOG_WARNING: used for errors that weren’t fatal but might cause some degraded functionality (for example, the fact that a request had to be retried before successfully completing).

  • BUILDBOX_LOG_ERROR: errors that, unlike those under WARNING, were fatal (for example, a request failing after exceeding the maximum number of retries).

  • BUILDBOX_LOG_DEBUG: verbose messages that give more information about the lower-level workings of a function.

  • BUILDBOX_LOG_TRACE: same as DEBUG, but an extra level of verboseness. This level should be used for things like loops that would generate multiple lines for a single function or event.

The following is an illustrative example:

int read(const std::string& resourceName) {
  BUILDBOX_LOG_INFO("Reading " << resource_name);

  int totalBytesRead = 0;
  while (reader->hasData(resourceName)) {
    BUILDBOX_LOG_TRACE("Data available, reading from " << resourceName);

    const int bytesRead = reader->Read(resourceName);
    if (bytesRead < 0) {
      BUILDBOX_LOG_ERROR("Error reading from " << resource_name);
      return -1;
    }

    BUILDBOX_LOG_TRACE("Read a chunk of " << bytesRead << " bytes from " << resourceName);

    ...


  }

  BUILDBOX_LOG_DEBUG("Read a total of " << totalBytesRead << " bytes from " << resourceName);
  return totalBytesRead;
}

Testing

<TODO>