Contributing to the codebase¶
Most contributions will be based on a the current sprint in gitlab(including bugs) and its user stories as indicated above, here are few things to keep in mind:
- Make sure you have read the README
- Contributions should be in form of merge requests
- All changes new and updates should have accompanying tests
- Merge Requests should include a brief description with a link to the ticket it fixes and a screen recording of how it works.
If you are contributing to the react codebase, you will need to have at least node v12 installed locally to be able to run checks before you commit and push, you can find out how to install node on different platforms here
For convenience, we recommend you add these aliases, you might need them quite often
if you are on a UNIX based OS, edit ~/.bashrc file and add these lines
alias rlint='docker-compose run --rm rails rake lint:fix'
alias rtest='docker-compose run --rm rails rake'
alias ytest='docker-compose run --rm webpacker yarn run test'
alias ylint='docker-compose run --rm webpacker yarn run lint'
You can customize aliases according to your liking, then when you want to run backend test, you can just type
We encourage to keep the codebase cleaner than you found it, refer to the following standards that we use in our codebase to get familiar with them.
Merge Request must be reviewed by 2 or more people. The engineer(s) who did the work and the reviewer. If the changes are simple enough the reviewer can approve and merge the changes without having the engineer who produced the code present.
The label 'Requires Rake Task' and the rake task command to be run must be added to the ticket, if the story requires a rake task to be run on staging or production
It is also encouraged for engineers to review each other's code once they submit a merge request and approve it, if it is good and ready to be merged.
Merge Requests where the release will affect what the user sees and/or interacts with need a UX sign off to ensure brand standards and the highest quality for experience for the user.
Each Merge Request should use the Merge Request template.
Only allow merge request approval once all items in checklist have been completed and checked by a second engineer.
If some of the checkboxes on the check list do not apply to the change then mark with them strike through and indicate that it does not apply with the text "Not applicable for this change". EX:
Videos on Merge Request should be shot in Loom or similar software that allows for controls on the video and should include a visual check for cringes by showing the browser being resized from XL to XS.
As a engineer, you MUST complete the 'Engineer Checklist' for the acceptance Criteria - Requirements. You must demonstrate to another engineer you completed all the acceptance criteria. A loom video link must be attached to the MR showing the engineer going through all the acceptance Criteria of the ticket to the reviewing developer. The reviewing developer can ask questions, clarifications, and additional tests to be conducted. If the checklist is completed, add this label
Staging::Verified. If not, leave a comment in the issue saying why it is not working and add this label
Staging::Bug and fix this issue. You can use initial branch to make the change, since branches get automatically deleted to add required fix and create an MR. If you closed the issue then reopen.