This is a guide to help our developers to understand the decisions made in the past that might affect Pull Request Reviews when they contribute to our services.
Before we proceed we should understand a few concepts that will be widely used in this small doc or in further diagrams.
- Domain: is the problem you're a trying to solve, like notification, basic entity management operations (aka CRUD) or face recognition. Although you might have multiple instances of a very single service (representing a domain), there is usually a single instance of it (the code) running per instance.
- Sub-Domain: is an attempt to split up domains into to smaller domains. It might cause you to have smaller services that works together to compose a single domain, as in the case of Product Order Services (Sales, Orders and ReportBuilder). In services transitioning from monolith to microservices a sub-domain might've been represented as a big module or a package acting as umbrella for smaller packages, as we can see in Sales Core module. There is usually a single instance of it (the code) running per instance.
- Component: a small behavior encapsulated in a way that is possible to reuse it any time it is required. It can be a class, or a few of them. It usually holds state (temporarily in some cases) and is often reused widely in the entire system, like a security or a request tracer module. When components get bigger they can be turned into a small service orchestrated by another component inside the sub-domain.
Due to troublesome experiences in a not-so-distant past, here a non-exhaustive list of what we've agreed to:
- Avoid the Entity Service Pattern whenever it causes scalability issues.
- Avoid creating graphs between dependencies, sub-domains or components (check Packaging Guidelines for further details)
- Avoid fuzzy, long, complex and hard to test methods (check Coding Guidelines for further details)
- Adopt Orchestration over Choreography whenever possible.
- Adopt CQRS only when its benefits are bigger than the cost of maintaining it.
- Mirgation scripts shall never break production, ensuring that a possible rollback will work once redeployed.
- Nobody should have direct access to production to avoid leaking AWS Credentials - or being hijacked - again. Our CI/CD will perform the modifications automatically to staging once ready. If it fails you can revisit it on your local machine, fix it and try again.
We've adopted a customized workflow based on GitFlow.
GitHub provides additional document on forking a repository and creating a pull request.
- There should an issue describing what have to be done so the feature is considered complete
- There should be one branch per feature
- Feature branches should match the pattern
feature/[0-9]+
, where the number at the end should be the same as issue number - Once the feature is considered stable, a Pull Request against
staging
should be created. - Developers should wait for at least one reviewer to get their PR approved
- Failed unit tests, or bug checks, will prevent the PR to be merged
- There should an issue describing what have to be done so the bug is considered fixed
- There should be one branch per bug
- Feature branches should match the pattern
hotfix/[0-9]+
, where the number at the end should be the same as issue number - Once the bug is considered fixed, a Pull Request against
staging
should be created. - The Pull Request should have the tag
bug
on it. - Developers should wait for at least one reviewer to get their PR approved
- Failed unit tests, or bug checks, will prevent the PR to be merged
Once your changes has been approved you can safely merge then to one of the main branches - staging
or production
. Any Pull Request merged to staging
will trigger the deployment pipeline, generate the Docker image and rollout a release deployment against the staging cluster. Likewise, Pull Requests merged against production
branch will promote them to production cluster.
Be aware that:
- to release all the features that has been stabilize in staging to production, a new Pull Request should be made from
staging
toproduction
branch - Pull Requests against production doesn't require review, being automatically closed once all checks has passed.
- if your Pull Request to staging contains the tag
Bug
, the patch will automacally release to bothstaging
andproduction
A few guide lines has been defined for our services. They might have their build process broken in case one of them is not followed. This would enforce us to apply the Single Responsibility Principle.
- Max number of lines per method: 15 - this includes the method signature, annotations and Js/Ts/KotlinDocs.
- Max number of methods within a class - 20 - this includes all of them
- Max number of characters per line - 120
- Max number of parameters per method - 4
We have adopted Lean Architecture as blueprint to organize our code base. Therefore, we should code respecting the following principles:
- Common Closure Principle (CCP) - gather into components those classes that change for the same reasons and at the same times (related to SRP)
- Common Reuse Principle (CRP) - don't force users of a component to depend on things they don't need (related to ISP)
- Acyclic Dependencies Principles - As soon as you begin using component, you should allow no cycles in the dependency graph. It's always possible to break a cycle with Dependency Inversion Principles.
- Stable Dependencies Principle. Some components are designed to be volatile. We expect them to change. Any of these should not depend on a component that is difficult to change. We should depend in the direction of stability. Again employing the DIP can help us to apply this principle breaking dependency on a stable component.