Please use descriptive full word variable names.
- In the lifecycle of most projects, fixing bugs and maintaining current features end up taking 50%+ of total engineering time.
- Our goal is to create a code base that is easy to understand, making fixing bugs and maintaining current features as painless as possible. We will have many engineers working with this code, and we want to be welcoming to engineers who are new to the project.
- Short variable names can often create confusion, where a new engineer needs to spend time figuring out what a short variable name actually means. (Ex/ Please use “person” instead of “per” or “p”.) For this project please use descriptive full word variable names.
- Fellow engineers should be able to zoom around the code and not get stopped with riddles created by short names.
- Please use full words instead of abbreviations (Ex/ Please use “Person” instead of “per” or “p”.)
- Please use lower case camel case ("camelCaseLettering")
- Please be aware that many variables are coming from our API server lower case separated by an underscore. (e.g., "google_civic_election_id"),
-
Asset names should be all lower case (ex/ "lower.png")
-
If a name has multiple words, use "-" between words in the name (ex/ "lower-case.png")
-
If sequential, put order word + number at start of name (ex/ "slide1-")
-
For fixed size images, include width and height at the end of the name (ex/ "-200x150"). Does not apply to svg files.
-
If an image is still a "first draft", lets add "-draft" to the name + version number (ex/ "slide3-connect-list-with-photos-draft1-300x370.png")
-
If adding images that are meant to be used in one limited area of the App, please put them in their own folder within "src/img/global".
Examples of good names:
slide1-ballot-simple.svg
slide3-connect-list-with-photos-draft1-300x370.png
Examples of names we avoid:
Image1.svg
There is no perfect code style. If you have worked at a few different companies or on a few different open source projects, you will have used a few different styles.
As of December 2018, the WeVote WebApp src directory contained 1073 files, that have been created and edited by dozens of volunteer software engineers. And we had dozens of different code styles in different files.
Inconsistently formatted code is less reliable code, so we decided to (mostly) put aside the issue of "what is the best code style", and to adopt the (very widely adopted) styles that Airbnb came up with for React and JavaScript. Airbnb's styles have been source controlled as an NPM project containing eslint configuration rules for Javascript, and a few related projects for React, and those rules have been incorporated in our WebApp.
If you use a IDE like WebStorm, (sadly WebStorm is not free), all the lint warnings will show up in amber, and lint errors will show up in red. Errors will block a git commit and must be fixed. If there is just no reasonable way to fix the lint error, or fixing the code is too risky for some reason, it is ok to suppress the error on the line where it occurs by adding a comment like this...
const element = findDOMNode( // eslint-disable-line react/no-find-dom-node
If your editor does not show lint warnings, running npm test from the command line will provide a list of the lint output.
The list might be quite long, so you may want to redirect it into a file that you can edit npm test > testOutput.txt
We value Software Craftsmanship for the WeVote project since it makes working on the WeVote code base …
- Fun since the product is composed of clean self documenting components that
- Do not repeat themselves -- each functional block has a component, or collection of components that can be widely used, or is completely unique.
- Are easy to rearrange on panels and pages, with less (or no) modifying of the components themselves for new uses or new UI approaches..
- Satisfying since changes have fewer unintentioned effects
- Easier to work on since, the architectural layout is clear, with properly named components and variables, and just enough comments to explain the area that are hard to understand.
- Reliable, since well defined components are partially self tested by being used in many different ways for many purposes.
- Do no harm: Stuff happens, but if you are not sure, ask before you commit.
- Software Craftsmenship: Leave every file they touch in better shape than they found it.
- In React JSX files: commit files that (as much as possible) comply with the “airbnb” preset for jslint (with our few modifications). If you understand the warnings in the files you work on, make small changes to clear the warnings in the areas you are modifying.
- In /WebApp/src/sass files: BEM Naming Conventions should be followed -- BEM Naming makes it straightforward to organize where to group styles together in an object-oriented fashion. (Ex/ We can use a style named
ballot__header__titleand find where it is defined.) - In Python: Clear all PEP8 warnings before committing.
- Do not copy classes and make small changes, instead examine existing classes and components to see how they modified for the new use case and then reused. If you absolutely have to copy, be sure to remove, or temporarily comment out, code that is not immediately in use.
- Commented out code should be rare and should include the date of commenting, and why it remains in place.
- After you have some experience with this project, carefully perform incremental refactoring to break apart monolithic components into smaller reusable components.
- After you have some experience, carefully perform incremental refactoring to rename classes, components and variables to match their current use.
- Don't commit refactoring changes that you are unable to test -- if you can't test it, or haven't tested it, don't check it in.
- Delete code that you made redundant. Cautiously comment out, or delete code that has not been in use for months. If unsure, add a comment with a proposed deletion date, a few months in the future.
- If while searching globally for a phrase, you find multiple matches, this might be a indication that a refactor is needed.
- In code reviews:
- Do not allow new slightly duplicated code into our codebase (unless there is a good reason for it).
- If variable or class names are will be meaningless to others, flag them.
- Overly complex code should be commented or simplified.
- Flag code style changes or variable conventions, that are unlike the rest of our code.
- Be on the lookout for abandoned, or duplicative styles. Clean them out when you find them. Before creating new styles, first look for styles that make sense to reuse.
- Simple is better than clever code, but if "clever" really adds value, add a note to help out the next engineer who will be looking at the code.
- Most React classes should be less than 300 lines long, at 500 lines consider breaking the class up into sub-components. If the the component’s render method contains multiple static blocks of React components and markup, consider moving them into a new React component.
- Ask before including new open source NPM projects. Our strong preference is to include projects that NPM rates as high in popularity, quality and maintenance. Avoid projects that haven’t been updated in months or years, as they can pose security risks.
- If you need to fix the open source projects we rely on, please make a pull request against that project.
- We use Jira to track issues with the WeVote website and mobile Apps, thanks to Atlassian, the makers of Jira, and their support for open source projects.
- If you're actively working on an issue, please mark it as "Fix in Progress" and assign the issue to yourself.
- Please only "take" one issue at a time, unless you speak to the Engineering Manager first.
- Please keep the team updated on the work you are doing by attending our weekly stand-up meeting, or post a note in the #coding Slack channel.
- We encourage "Find it-Fix-it" for issues you find, if you are able.
- If you would like to report a bug for another engineer to fix, please report it in the #engineering-bug-reports Slack channel
- Anyone can report issues via our support system
- Please provide a link to where you saw the issue
- Describe the issue with as much detail as you can -- details really help us reproduce the problem so we can find a fix
- ...and THANK YOU for taking the time to report the issue. We appreciate it!
- Want to join our team? We are 100% volunteer, and volunteer opportunities are here.
We use SemVer for version numbers. More info: Versions: Release Names vs Version Numbers