-
-
Notifications
You must be signed in to change notification settings - Fork 724
Code Conventions
Firstly, code conventions are enforced by Rubocop, in order to improve code consistency and quality. Please ensure you check with Rubocop before each commit (consider adding it to a pre-commit hook).
This pages lists additional conventions that are not verified by Rubocop, but we agree to follow.
Use existing code convention patterns where appropriate. Including:
Please adhere to the naming convention when introducing new query-related services in the app/queries
folder. This consistency will streamline code readability and maintain a clear understanding of the purpose of each service.
See this guide: Database-migrations
j
is an alias for escape_javascript
.
When you have to insert an arbitrary Ruby string to a Javascript or Angular expression, this will ensure that characters are properly escaped and will not cause parsing issues or syntax errors.
# Bad
# JS syntax error when description contains a single-quote
"{description: '#{enterprise.description}'}"
# Good
# Single-quote is escaped
"{description: '#{j enterprise.description}'}"
For example, when asserting text that is translatable, use plain text instead of the I18n.t
call.
Bad:
expect(json_response['errors']).to eq I18n.t('admin.order_cycles.bulk_update.no_data')`
# Does not catch typos in translation values, especially when including variables.
# Example: "Welcome #{user.name}" is wrong while "Welcome %{user.name}" is right.
#
# Does not catch duplicate translation keys in the locale. If you accidentally define
# a key twice in a YAML file then only the second one is applied. If you wanted the
# first translation defined in the file then the test passes while the app prints the
# wrong text.
Good:
expect(json_response['errors']).to eq "Hm, something went wrong. No order cycle data found."
This format is much easier to read. It's also more likely to find typos and bugs this way. You still need to update a spec when changing the text but that is a user-facing change. In return, you don't need to update the spec when you move or rename translation keys.
Read more about the discussion
In app/serializers/api/address_serializer.rb we want to add the country to the address so we do:
def country_name
object.country.andand.name
end
If address.country is not eager loaded, this code will trigger a N+1 query.
TODO: how to avoid this
Using has_many declarations in serializers is one of the most common causes of N+1 queries in code.
TODO: how to avoid this
Development environment setup
- Pipeline development process
- Bug severity
- Feature template (epic)
- Internationalisation (i18n)
- Dependency updates
Development
- Developer Guidelines
- The process of review, test, merge and deploy
- Making a great commit
- Making a great pull request
- Code Conventions
- Database migrations
- Testing and Rspec Tips
- Automated Testing Gotchas
- Rubocop
- Angular and OFN
- Feature toggles
- Stimulus and Turbo
Testing
- Testing process
- OFN Testing Documentation (Handbooks)
- Continuous Integration
- Parallelized test suite with knapsack
- Karma
Releasing
Specific features
Data and APIs
Instance-specific configuration
External services
Design