Conventions

This is a compiled list of the conventions the team agreed upon over the past 6 months. It should not be considered complete, but at least provide a short introduction for new team members.

Branching

• Master is the integration Branch
• Feature branches should branch off master and be named bugfix|feature/<TICKET-#>-short-description
• After code freeze, master is merged into release, release gets deployed to staging
• Fixes to the release candidate are prefixed with bugfix but branch off release branch
• Tbd: Either we merge to prod branch or we stick with release and promote a build via Gitlab
• Release can be merged to master at all times, master to release only at the end of a sprint
• Bugs on production start off the release branch, branches are prefixed hotfix/. Don't forget to merge the release branch back to master when you finished the bugfix.

Pull Requests

1. Be polite and objective. You can make subjective statements, but please phrase them accordingly.
2. Every pull request needs at least two approvals before it can be merged. If for some reason more than two developers are reviewing a PR, wait for everybody to conclude their review.
3. Don't push to master without a pull request. Urgent hotfixes are excepted from this, but you should still let another developer sanity-check your changes, if possible.
4. Only the author merges her/his pull requests. There might be some hidden dependencies only the author is aware about.
5. Avoid assigning other developers to your pull requests. Every developer with enough time should assign her-/himself.
6. Reviews are done by intermediate developers and above, that are part of the project for at least a month.
7. Optional change requests are prefixed with Suggestion:. The author doesn't have to implement those, but he/she should seriously consider them.
8. Avoid rebasing after creating a PR. There is no benefit in this
9. Only create PRs for code that is ready to be merged (No WIP PRs):
   1. The code complies with our code conventions and is architecturally sound
   2. No errors or warnings in the console (both browser and shell)
   3. No failing test cases
   4. It fulfills the acceptance criteria.
   5. It works in all environments, in which it is supposed to run in (e.g. don't use browser APIs during server-side rendering)
   6. Avoid unnecessary formatting changes. Smaller diffs are easier to "scan". Changes, that improve the readability are considered necessary.
   7. The author should go through every line and judge it from a "reviewer's perspective".
10. The reviewer should:
11. ... identify errors that will cause an issue in production
12. ... verify that the changes being made align with stated goal of the code change and the acceptance criteria.
13. ... verify that any changes align with the team's coding standards.
14. ... look for anything they personally disagree with and phrase it accordingly (e.g. IMHO). PRs aren't only meant for the code at hand. They play a big role in aligning the team and teaching more junior team members.
15. Add 'Closes #' in first line of PR description to automatically reference and close linked issue.

Code Conventions

1. We tend to follow the principles of functional programming and clean code (by Robert C. Martin). If you do not have a copy available, one can be provided.
2. Naming conventions:
   • If not otherwise specified: camelCase
   • Components and Interfaces in PascalCase
   • Filenames in kebap-case
   • Interfaces for component props: [Component Name]Props
   • Constants in CONSTANT_CASE
   • Functions should start with a verb
3. Avoid any wherever possible.
4. Avoid left-over console.log's.
5. Avoid deeply nested code. Add guard conditions at the beginning of a function and exit early.
6. Avoid state.
7. Avoid TODOs without Jira ticket reference.
8. Avoid reformatting of imports.
9. Avoid ternary operators, that are longer than three lines
10. Avoid nested ternary operators
11. Connectors never throw Exceptions, only shops may do this to have a 500 page rendered
12. Every new dependency needs to be justified and vetted.
13. Every dependency, that isn't required at runtime should land in devDependencies.
14. Use arrow-functions whereever possible. Implicit returns are preferred.
15. Prefer if ... elseif ... over if .. if ..
16. Commit messages start with the ticket ID (e.g. "SBOT-1337: ...") and use present tense. Urgent hotfixes, that don't have a ticket should be prefixed with SBOT-Fix.
17. Use prettier and auto-format your code
18. Add a newline between all top-level statement blocks to improve readability.
19. Every new backend feature should be covered by a test.
20. Files should be named after the default export. If a file doesn't have one, the filename should reflect the shared context / topic.
21. Do not duplicate the code in the documentation. Do not explain the what, explain the why and warn others about potential pitfalls.
22. Co-locate code: Keep the definitions/files/assets as close to its usage as possible
23. Do not force push to master.
24. Strive for consistency. So even if there is no explicit rule: When in Rome, do as the Romans do

Ticket Workflow

1. The tickets are ordered by priority. Try to take the ticket with the highest priority. Fast lane tickets have the highest priority.
2. Don't reserve tickets: Avoid assigning tickets to yourself, if you still got uncompleted tasks to do.
3. Blocked tickets should be marked with a flag.
4. Try to get your tickets done before the code freeze. See your calendar for the exact date/time.
5. The time between code freeze and the next sprint should primarily be used for testing and bugfixing
6. When starting a ticket move the issue in 'In Progress' issue board lane.
7. Move ticket to 'Code Review' lane after creating a new PR
8. Reference 'Closes #' inside your PR description to auto close the issue on successful merge.