Skip to content

Arch Forum 2025-01-23

Participants: Backend devs, Andy, Magnus, Victor and Zak

Agenda

  • bank-localization
  • Storage accounts
  • Code reviews

Summary

bank-localization.

Previously we had a vision that all localizations would live in the bank-localization git repo (instead of spread out in each area in the app-locales folder like now). However, this vision was only implemented for be-remittance and be-sendtab. With the partial implementation which only created confusion and extra work for those areas Growth/Services saw that it was best to revert this back to use app-locales in each area instead.

The original idea might still be a good one, and we might come back to it at a future time if we see the need again. However, at the moment there are other more important issues to prioritize.

Storage accounts

Normally a area should have 0 or 1 storage accounts. If the area needs to store more than one type of data, it should use containers/folders within the storage accounts. https://dev.azure.com/MAJORITY/Documentation/_wiki/wikis/Main/521/Storage-Accounts-(blobs)

A couple of comments were raised:
- Could/should we mark storage accounts that are public somehow to easier manage them?
- If redesigned from scratch it might be better with a single storage account for all of backend. However, we are not there currently.

Code reviews

The last topic were a discussion around code reviews. Below is cleanup raw notes from this. The plan is to have a more structured followup later.

Why do we have reviews?

  • Required for compliance
  • Improves code quality
  • Sanity check
  • Knowledge sharing by be "forced" to look at other peoples code in other areas.
  • Knowledge sharing because you learn by getting comments
  • To catch mistakes
  • You get reminded that you "forgot" adding tests

When to not have reviews

  • some kinds of reviews are not useful, like config changes
  • when PRs are scripted.
  • The github yaml files?
  • When project refs are updated, or other changes were only check if compiles/test pass
  • localization file changes, and configuration?
  • Have to be careful about some changes, but it can have big negative result if bad config (like tx type)

What is a good review?

  • code works or not / is the code correct
  • cleanness, how it works, could it work better
  • is the new/changed code tested?
  • a review that is approved!
  • the reviewer should understand what this code change is about. Easy to not do. read jira ticket.
  • subjective / style things. these are less important and shouldnt block the pr
  • should also check architectural patterns/ design
  • its difficult when reviewing code from the owner of an area, especially when you are not an expert of that thing.

Additional notes

  • Should you need to run the code (debatable)
  • A somewhat common practice for more indepth/bigger changes is to have a pre-review / design discussion before the code is ready for review
  • Are the branch name important?
  • What about the Commits? Some devs like Rasmus like to review commit by commit if the commits are well organized.