This blog post is not for the average PayrollHero user, fan or follower, and not for the faint of heart – we must warn you that it’s quite technical!
However, we would love feedback/comments from you developers/programmers/techies out there!
As a bit of background, the PayrollHero Engineering team has a mandatory weekly meeting with every Engineer in attendance. We use that meeting as a forum to work out any issues we’re having, or discuss stuff that’s been bugging us, and even set policies.
We wrote out some of the best practices we already have in use, and some more that we may want to adopt, and then we voted them in as standards for the team, which will essentially becomes our constitution. Our goal is that these Best Practices will help us create higher quality code, and at the same time avoid some typical internal conflicts.
This list is a work in progress, and every other week we agree to add or modify items. I’ve pasted them here below the break since it’s quite a list!
We’d like to invite feedback on this list. Where this stacks up against stuff your developers/programmers/engineers do? Your comments are welcome, please let us know what you think.
Workplace/space best practices:
- We always work out of an office
- We always use pairing as a way to interview people
- We always pair with new engineers to help them onboard faster
- We always do daily huddles
- We always have weekly engineering meetings to help tackle bigger problems and create a forum for all engineers to have an opportunity to speak up
- We always set up time boxes for different activities we have to do within a week (eg, 30 min each morning to deal with emails/ticket updates, etc.)
- We always maintain up to date README files on all projects in order to make it simple to get going with our various projects.
- We aim to use the GitHub wiki to document more complex systems.
- We always involve engineering early in the ticket creation process because we understand engineering input is just as important as business or user experience.
- We always break up larger tickets into smaller sub tickets.
- We always include the 3 stewards (engineering, user experience, business) in ticket creation (for enhancement, feature and involved bug tickets).
- We always expect cucumber-like set of steps in ticket definitions to act as the acceptance criteria or reproduction steps for bugs.
- We always work in small deliverable chunks, even when working on multi day tickets, helping to stay in a working state at all times.
- We always use subtasks to track small deliverable chunks of any ticket.
- We always pair when working on tickets longer than 2 days.
- We always rotate pairs when working on tickets longer than a week.
- We aim to create a functionality design plan and discuss it with one or more team member before writing code, especially for more complex tickets.
- We aim to seek continuing feedback from a team member if there’s a need for a direction change on a ticket.
- We aim to be Scouts, not Marines, leaving the code base a better place than we left it.
- We aim to avoid pre-optimization unless it’s proven to be needed.
- We always issue GitHub pull requests for any changes going into our applications.
- We always link to the ticket in pull requests.
- We always mention the CI status of the branch being pull requested.
- We always do code reviews between working on tickets or in natural breaking points.
- We always care about the status of CI and help to keep it green.
- We always take responsibility for the entire code base and don’t pass blame when an issue arises.
- We always use checklists for standard processes, and we follow those checklists.
- We aim to have code reviewers commit their suggested changes and issue them as a pull requests against the merging branch.
- We aim to use brakeman (or other static code analysis tools) to help us avoid security issues.
- We aim to automate repetitive tasks.
- We always continuously rebase our feature branches with master to avoid merge issues
- We aim to make code work first before refactoring it into modules/classes to avoid unnecessary complexity
- We always expose only the necessary methods on our objects.
- We never use singletons for anything that has any state between threads.
- We always write self documenting code.
- We always prefer self documenting code over comments and as such use them as the last resort.
- We always use descriptive method names that do not use abbreviations.
- We always telegraph the methods mutable nature using ‘!’ methods.
- We always include a short comment on top of a class explaining its purpose/motivation.
- We always factor out resources that would grow past the basic REST methods, instead implement new sub resources which use the REST method set.
- We always create DB unique indexes matching uniqueness constraints on models.
- We always create DB indexes for foreign keys.
- We aim to write small methods (<= 7 LOC).
- We aim to use constants only as the last resort.
- We aim to avoid unnecessary assignments and variables.
- We aim to decompose fat controller amd models into decorators/concerns/concerns or similar.
- We aim to keep objects only concerned with one thing.
- We aim to not use hashes where an object with behavior would make more sense.
- We aim to return self on void methods.
- We always write our tests first.
- We always create tests covering all business logic.
- We always test all public methods.
- We always create a test covering any bug fix, even if its a guess, we create a test for that guess.
- We aim to extract code into modules if we ever feel the need to test privates.
- We aim to use cucumber features as acceptance tests only, we don’t think there is value in writing overly complex cucumber steps to force complex integration tests.
- We aim to use rspec integration tests to cover more complex cases for deeper integration tests.
Phew – have you made it this far? You can see that we’ve developed these lists over quite a lot of trial and error. Do you see anything you would add/change, or have any comments about it? We’d love to hear from you, if you do!