Contribution Guide

This page is meant to function as a set of general principles for the code that is contributed to the Network Canvas suite of apps (Network Canvas, Server, and Architect). These principles are designed to help us meet our overall goals: to have software that is easy to develop, easy to extend, and easy to debug.

Contributions from any team members are welcome, but please discuss your idea on Slack first!

---

1. Code should be self-documenting, and as far as-is possible, self-explanatory. This means that we try to keep our kewl new ultra-concise one liners (that uses uncommon or non-standard language features) away from the codebase. Sometimes we all come up with something neat but hard to understand at a glance. If this happens, a short comment will help the next person along get what you were doing without having to turn their brain into a JS compiler. In general, we would rather our code be inefficient and readable, than technically perfect but hard to grok.
2. Components should gracefully handle invalid/incorrect properties wherever possible. Incorrect properties should not cause a component (or the app) to fail. Where possible, handle those errors gracefully and show something indicating an error state. This is less of an issue in React 16, because you can use error boundaries. If you are writing a new component, please try to use them.
3. No speculative implementation. This is for a few reasons. Firstly, our time estimates for features depend on keeping to the existing scope. Secondly, your extension to the planned functionality could interfere with features that we actually plan to implement at a later date, that just haven't yet been fully spec'd. Thirdly, it makes our code less readable, as the documentation won't precisely reflect the functionality of individual components or interfaces.
4. No committing to master. If you are under pressure, or need to fix something that quickly, something else has gone wrong in our process, and we should talk about it. Any commits to master that simply have to happen should be accompanied by an explanation on Slack.
5. There are no stupid questions. No one is expected to know everything. Speak up early if there's something you're not sure about, or even if you just want a sound board to help shape a new idea. Our team has varied skills, and you are not expected to be the master of everything!
6. Don't be afraid to refactor! Just because a set of functionality was implemented a certain way at a certain point in time (and by someone you think knows more than you) doesn't mean that it is still the best way to do it now. If you are implementing functionality that changes the scope or meaning of one or more components - please consider refactoring them!
7. Write tests for your components as part of your PR. For us to merge a PR, it has to have tests that cover the new functionality. Try to write tests that cover the obvious stuff (invalid/incorrect properties, rendering correct markup, etc.), but that also reflect the way your component will be used. If you are not an expert on testing and want some advice, or if you just want to soundboard an idea, ask!
8. Feedback on PRs and in code reviews should be honest, but friendly. Recognize the hard work of your fellow team members, even when you disagree with their implementation. Adopt 'good faith' when you find seemingly obvious mistakes - everybody makes them from time to time. These things may seem irrelevant in the context of a technical review, but politeness, praise, and positivity help all of us to grow and take on board feedback more readily. This in turn will make our code better, and our team happier.