Project

General

Profile

Task #3898 » CONTRIBUTING.md

Draft 1 of the contributing documentation - Nicholas Gordon, 01/04/2017 02:43 PM

 

Contributing to NDN Projects

Getting Started

The NDN group of projects is very glad you are interested in
contributing! NDN (Named Data Networking) is composed of many
projects, with multiple universities across the world
contributing. This documentation will help you understand how we
work, and what we expect from contributors.

Code of Conduct

The NDN projects adhere to the Contributor Covenant, located in
CODE_OF_CONDUCT.md. By contributing, you are expected to also adhere
to this code. If you feel someone has breached this code of conduct,
please email Lixia Zhang
(lixia@cs.ucla.edu), Beichuan Zhang
(bzhang@cs.arizona.edu), or Alex
Afanasyev (aa@cs.ucla.edu).

What can I do?

The NDN group of projects continually needs help with, among other
things:

  1. Implementing new features
  2. Writing fixes for bugs
  3. Writing documentation for features that have recently changed or that change quickly. Sometimes people forget to update documentation!
  4. Code review

Finding Documentation

The NDN group of projects maintain a community Redmine wiki
(link here) for issue tracking and
documentation hosting. Consider the Redmine to be the central hub of
activity. In particular, all design discussion and decisions will
either occur or be copied there.

Why did they do it this way?
When writing code, if you ever have questions about why
some design decision was made, the best approach is to use git blame
on a file to inspect the last commit for some line of code. In this
view there will be a commit hash in hex (e.g. 0ab12e3a) in the first
column. Using our example, git show 0ab12e3a would show the commit
made for that line. In the commit view there should be a reference
number (e.g. refs: #1234). Using this number, you can find a
pertaining issue on the Redmine
(e.g. https://redmine.named-data.net/issues/1234).

Additionally, in some cases there are published papers you can read to
in order to gain a better understanding of the project. Searching for
these papers is not difficult; in many cases you can find a pertaining
paper by using the search on the main webiste
(link here) to look for a few
keywords. Usually the title of a project will be a good keyword.

If you cannot find an answer to your question, the best place to go is
the mailing
lists. (link here)
There are multiple different lists for different interests, so be sure
you are mailing the right list for a quick response.

Contributing Guidelines

Repositories

All NDN projects are hosted on GitHub
(link here), using git for VCS. If
you are unfamiliar with git, some kind of tutorial on git should be
your first
step. (such as this one)

There are a lot of different projects, so take some time to look
through them for ones that pertain to your interests. NDN projects use
Gerrit (link here) for code review
purposes.

Occasionally there will be other repositories used privately to test
changes, but this is uncommon. In general, you will not need to refer
to these repositories.

Style

Most NDN projects are written in C++, and there is a style
guide. (link here)
In general the NDN style is similar to the GNU style, but there are
some significant changes. This style guide is not exhaustive, and in
all cases not covered by the guide you should emulate the current
style of the project.

If some code that is not in style is changed, it should be brought
into style. The goal is 100% style compliance. This is important
because many of the developers only very rarely meet in person, so an
understanding of common intent is important, and eases collaboration.

Some NDN projects are written in Python or JavaScript, but there are
not official style guides for these languages. (?) In these cases,
you should follow the current style of the project.

Commit Messages

Commit messages should:

  • Have a short, descriptive first line, starting with the module they change. A good rule of thumb is a maximum length of 80 characters.
  • Include Redmine issue numbers. The exact syntax is given below.
  • Be written in an imperative mood. E.g. "Make foo a bar" and not "Foo is now a bar"
  • Use the present tense. E.g. "Make foo a bar" and not "Made foo a bar"

Unit Tests

Every patch needs to have unit tests that accompany them. For C++, we
use the Boost unit test framework to help us
out. (link here) Note that this link
points to the newest version of the Boost Test library documentation,
and you may need to refer to older documentation if you are using an
older version of Boost.

When designing tests, a few things need to be kept in mind:

  1. Unit tests are design tools, not debug tools. Just because your code passes some unit tests does not mean it is bug-free. Unit tests are tools to convince you that your code does what you think it does.
  2. (?) It can be difficult to know when you should test something. If you find that you are having a hard time designing a test for something, ask yourself whether it is because it doesn't make sense to test what you've just written, or if it's because the way you designed it makes it difficult to test. Consider a second look at your design if you think it's the second one.
  3. The method that gave the world unit tests says that they should be written before the code they test, not after it. Consider this when designing a patch, and you may have better success.

Writing unit tests using the Boost framework is quite simple, and you can refer to existing unit tests for examples. (this is a good example)

Tracking work on Redmine

As mentioned above, Redmine is the organizational hub of the NDN
projects. As such, extensive use of it is made, and learning how it
works will help you. In particular, there is a workflow associated
with Redmine. Generally it is:

  1. An issue is reported by someone. At this point they should describe as much about the issue as possible, including its relationships to other issues, target version, and priority.
  2. Design discussion about the issue occurs, and the issue is either accepted or rejected.
  3. Someone either is assigned to, or assigns themself to work on the issue, setting the status to "In progress."
  4. Once all patches are cleared for merging, they either can be merged, or held for feedback on the changes, at which point the issue status should become "Feedback."
  5. After all patches have been merged, the issue should be set to "Closed."
  6. The changes are uploaded to Gerrit for review. Once all changes pertaining to a change are on Gerrit, an issues status should change to "Code review."

Redmine provides other facilities for managing your work on NDN
projects, too, such as time logging and time estimation.

Gerrit: Uploading Patches and Code Review

As mentioned above, NDN projects use Gerrit
(link here) for code review. This is
a web-hosted, open code review platform that allows for interactive
code review, rebasing, and cross-linking with the Redmine. Extremely
important to note is that on any Gerrit change, the refs: #...
should be clickable links that take you to the Redmine for design
discussion.

First-time Setup

The first-time Gerrit setup goes like this:

  1. Add Gerrit as a remote location to your local git repo. (how to do this)
  2. Log in to Gerrit. You can authenticate using many different methods, including GitHub OAuth.
  3. Set up your Gerrit credentials. This will depend on how you configured your Gerrit remote in step 1. Among other things, you need to set up identities so that the email on your Gerrit profile matches whatever email you will be committing with on your git repo.
  4. Set up commit hooks. You can do this by attempting a push (git push gerrit HEAD:refs/for/master if your remote's name is gerrit). You can then copy-paste the curl command to get commit hooks for this repo. Whenever you write a commit message from this repo, it should automatically assign the commit a Change-Id.

Uploading patches

All patches should have a corresponding Redmine issue that they can
reference. If you search the Redmine and notice there is no relevant
issue for a patch you are writing, please create an issue first. You
will need a Redmine account, which can be created there.

After writing some changes, commit them locally as normal. Then, run
git commit --amend, which should insert into your commit message a
unique Change-Id, provided by the commit hook installed earlier.

The anatomy of a typical commit message is like this:

//| Color this? Make it in a box? How to do that?

rib: Make foo a bar instead.

refs: #1234, #1235

Change-Id: ...

Explaining this:

  • rib is the module that the commit affects. We want this because it lets someone know at a glance what part of the project it changes. For some projects, there will be only one module or only very small other modules. This practice should be observed in those cases, too.
  • #1234 and #1235 are the Redmine issue numbers. Gerrit transforms these into clickable links, and it is useful to reviewers to gain background understanding of the issue.
  • Change-Id: ... should be filled in automatically. It is used by Gerrit to track changes.

Once you have a commit message you are happy with, simply run git
push gerrit HEAD:refs/for/master
to upload your patch.

Note: Gerrit separates commits into patch sets by the unique
Change-Ids. As a result, it is important that you either:

  1. Amend your commit with any new changes using git commit --amend, or, more complicatedly
  2. Collapse your various commits into one with git rebase -i <initial commit>, ensuring that the ultimate Change-Id in the commit is the one on the patch set on Gerrit.

If you do not do this, what will happen is that each commit will be
interpreted by Gerrit as a separate patch set. This is probably not
what you want.

Code Review

Dealing with Code Review

It is important to remember that code review is about improving
the quality of code contributed, and nothing else. Further, code
review is highly important, as every line of code that's
committed comes with a burden of maintenance. Code review helps
minimize the burden of that maintenance. Consider that when you
are receiving comments, those comments are influenced by two
things:

  1. How the reviewer personally feels about the change ("Would I be happy to see this code in a year?" or "Would I be happy if I wrote this?") and
  2. How the reviewer feels the change abides by style and usage requirements ("I may not personally mind, but we have to do it this way.")

Remembering these things when reading comments helps to separate
what can feel like needless negativity.

Doing Code Review

Code review is extremely important! We need every bit of code
review you can give. In many cases this is the bottleneck for
new contributors who do not fully understand how certain
language constructs should be used, what best practices are,
etc. In these cases it is important to give constructive
feedback.

Minimally, a review must include:

  • A score (usually -1, 0, or +1)
  • An "itemized" commentary on each objection you have, or a justification for a whole-change objection.

Optimally, a review should include:

  • A useful explanation of why you object to some item. This is more important than it first appears. Consider that although you may have been writing code since before you could count, not everyone has. Some things may not be obvious to others, and sharing your knowledge is key to making an open-source project work.
  • Comments about good design decisions. These help motivate developers when otherwise a review is entirely negative comments.

Expediting the Code Review Process

If you observe that code review takes a long time, there are a
few things that you can do to to expedite the process:

  1. Think about language best practices.
  2. Ask yourself how you would feel about this code, if you saw it "in the wild."
  3. When making code decisions, ask yourself "why shouldn't I do it this way?"

Jenkins

As part of the code review system, every patch set is automatically
submitted to Jenkins
(link here). Jenkins is a
continual-integration platform that allows for automatic testing of
every change to ensure that it builds on all target platforms.
Interacting with Jenkins is not usually necessary, as the pickup of
patchset changes in Gerrit by Jenkins is automatic, and Jenkins
automatically posts back with the results. Typically the only
interaction needed with Jenkins is when some kind of glitch occurs and
a build needs to be retriggered.

Since there are many projects competing for processor time on the
build slaves, it is recommended, if possible, that you verify locally
that your code compiles. In some cases reviewers want to wait until
Jenkins verifies the build before giving code review, so you can
expedite the process for yourself this way.

(2-2/2)