Skip to content

Instantly share code, notes, and snippets.

@gorgonical
Last active January 19, 2017 16:49
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save gorgonical/d484280cd2597a5337658e9981b38f42 to your computer and use it in GitHub Desktop.
Save gorgonical/d484280cd2597a5337658e9981b38f42 to your computer and use it in GitHub Desktop.

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 us.

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) for issue tracking and documentation hosting. Redmine is the hub for design 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 website 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 There are multiple different lists for different interests, so be sure you are mailing the right list for a quick response.

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. After every useful chunk of progress, notes are made on the issue and the progress percentage is updated to reflect this.
  5. 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."
  6. After all patches have been merged, the issue should be set to "Closed."
  7. 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.

Contributing Guidelines

Repositories

All NDN projects are hosted on GitHub, 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 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. 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.

There is a partial styleguide for Python. It applies many of the provisions from the C++ guide, in addition to some other Python-specific things.

Some NDN projects are written in JavaScript, but there is not an official style guide for this language. In this case, 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. 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. More information on this is available in the [NFD Dev Guide](dead link.)

When designing and writing 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. Keep unit tests in mind when writing code, and you may have an easier time writing the tests.
  4. Write the test for a completed module/unit of code before moving on. Doing this will ensure that the test gets written, and will help you think about the interface that you need to examine. If you don't, you may forget exactly what each piece is responsible for when you're looking at the whole system afterward.
  5. Separate your I/O. It is very hard to test, so I/O should be isolated if possible. Consider something like separating a function that invokes I/O into two functions: one that does the I/O and calls the other one, which takes that I/O result. This will make it easier to test that second function than if they were combined.

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.)

Gerrit: Uploading Patches and Code Review

As mentioned above, NDN projects use Gerrit 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.

  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.

    This shows what the identities panel looks like. If you do not see the email here that you have configured git to use, you cannot push to Gerrit. Web page showing what the "identities" panel looks like

    In that case, add it under contact information panel. Web page showing what the "contact information" panel looks like.

    Gerrit itself has extensive documentation regarding error messages, and this identity-based one is by far the most common. Its specific documentation can be found here.

  4. Set up commit hooks. You will encounter an error if you attempt to push to Gerrit and haven't done this. We recommend that you set up commit hooks by copy and pasting this into your terminal: curl -kLo `git rev-parse --git-dir`/hooks/commit-msg https://gerrit.named-data.net/tools/hooks/commit-msg; chmod +x `git rev-parse --git-dir`/hooks/commit-msg). Whenever you write a commit message from this repo (i.e. the base directory of the project), it should automatically assign the commit a Change-Id.

    The prompt to set up the commit hooks should look something like this: Screenshot showing the gerrit commit hooks prompt

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:

The anatomy of a good git commit message

Explaining this:

  • docs 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.
  • #3898 is the Redmine issue number. Gerrit transforms these into clickable links, and it is useful to reviewers to gain background understanding of the issue. You can have multiple by separating them with commas.
  • 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:

  • 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
  • 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 and Writing Comments

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.

Writing comments is somewhat counter-intuitive on Gerrit. If you are signed in through a modern browser, you can leave a comment in a file by clicking on the line you want to comment, or by clicking on someone else's comment. After typing, click the Save button there. After navigating through the patch set with the arrows at the top-right of the Gerrit UI, you then must click the up-arrow to get back to the patchset. At this point your comments have not been made yet! You must then click the Reply... button, and assign a score. If you correctly saved your comments, they will be shown at the bottom of that box. Once you click post, the comments will be made public to others.

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?"

Responding to Code Review

There are a few things to remember when responding to code review, including:

  • Don't click "Done" on a comment if you agree with a comment and are updating the code accordingly. These comments are essentially useless, and only clutter the discussion. Gerrit has a convenient mechanism to check the differences between two patch sets, so reviewers are expected to check that their comments have been addressed.
  • Don't blindly follow what is suggested. Review is just that: a review. Sometimes what is suggested is functionally equivalent to what you have implemented. Other times, the reviewer has not considered something when writing their review, and their suggestion will not work. Lastly, if you disagree with a suggestion for some other reason, feel free to object, but you may be asked to provide an explanation.

Jenkins

As part of the code review system, every patch set is automatically compiled and tested on multiple platforms using our instance of Jenkins, the continous integration system. 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.

It is expected that code is checked locally. Reviewers may wait until Jenkins checks the code before doing a more functional review of the code. Since Jenkins checks can take a while, you can save some time by checking yourself first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment