Contribution Guidelines¶
In this section we'll review some contribution guidelines, best practices, and internal development and project organization tasks.
The Suite Monorepo¶
This repository includes the Suite Framework and Suite Services. These Services may be developed by different teams, hence this is a monorepository.
The monorepo is a way to organize the code. That's it. However, having different teams working on the same repository at the same time can be complicated, if not done properly. Automation and efficient build systems are required.
Different parts of the repo have different owners. The owner is the PM of the team in charge of that project. The owner is the one who has the final say when merging a PR that affects his project. The current list of owners is:
- Suite Framework: Leonardo Chaia (lchaia@itsynch.com)
framework
modules
runtime
samples
templates
tools
- Suite Framework and its Modules documentation.
- DataSync: Alan Asmis (aasmis@itsynch.com)
DataSync
- Datasync and its Modules documentation.
- Amos: Lucas Kloster (lkloster@itsynch.com)
- All
Amos
related .NET services
- All
- Users: Lucas Kloster (lkloster@itsynch.com)
- All
Users
related .NET services
- All
- Positions: Lucas Kloster (lkloster@itsynch.com)
- All
Positions
related .NET services
- All
- Departments: Lucas Kloster (lkloster@itsynch.com)
- All
Departments
related .NET services
- All
- UxRoles: Lucas Kloster (lkloster@itsynch.com)
- All
UxRoles
related .NET services
- All
- GeneralArrangements: Lucas Kloster (lkloster@itsynch.com)
- All
GeneralArrangements
related .NET services
- All
- Scheduler: Lucas Kloster (lkloster@itsynch.com)
- All
Scheduler
related .NET services
- All
- Components: Lucas Kloster (lkloster@itsynch.com)
- All
Components
related .NET services
- All
- ServiceOrders: Franco Leonardi (fleonardi@itsynch.com)
ServiceOrders
- ServiceOrders modules and their documentation
- Inspections: Adhemar Bouchet (abouchet@itsynch.com)
Inspections
- Inspections modules and their documentation
Tradeoffs¶
- Suite Framework and services can evolve together at the same peace.
- All projects use the same dependency versions for third party libraries. For
.NET read this, for Angular, it's the root
package.json
. - All projects directly reference other in-house projects.
- Breaking changes are applied to all projects by the dev performing the
change.
- Help might be requested from other teams to perform the changes.
- Changes to other team's source code is reviewed and approved by the corresponding owner.
Branching strategy¶
We follow Microsoft's Branching Guidelines.
We have a main
branch where code can only be merged through a Pull Request.
Every PR must pass automated test suites and Definition of Done.
The contents of main
are always production ready and comply with the Suite's
Definition of Done.
Pull Requests¶
Contributions to main
can only be merged through a Pull Request. When creating
Pull Requests, it is important that you follow the guidelines stated here. The
purpose is to make reviewers job easier, and to prevent naming collisions when
working with other teams on the same repository.
Keep up to date against main¶
As a general comment for creating PRs, make sure that you are always working
with the latest version of the main
branch. If keeping up to date against
main
causes you "a lot of merge conflicts", consider that the longer you wait
the worst it will be.
Something else to keep in mind is that for PR branches the Commit History does
not matter, since the PR will be squash merged into main
in a single commit.
However, keep it sane. The PR branch commit messages will be added to the final
commit's description in main. So please, explain WHY the changes in the commit
and follow
Conventional Commits
convention.
Continuos Integration (CI)¶
When PRs are created, a continuos integration process starts. The PR will be built and tested automatically. Builds are run on the ITsynch CDU office using Linux agents.
Multiple checks are done during the CI process. All of them need to succeed for the PR to be merged.
You may notice that not all builds will be triggered every time, or that builds will finish faster some times. This is due to the CI building only what is affected by a set of changes.
PR Review Process¶
When a PR is created, Azure DevOps will automatically assign reviewers based on the modified files. The PR won't be merged until all required reviewers approve the changes.
After all reviewers approve, and all CI builds pass, the PR is ready to be Completed. At this time, if new changes are pushed, at least one of the reviewers must approve the PR again.
When creating a PR, you have the option to create a Draft Pull Request. Please create PRs in draft until you consider them ready to be reviewed.
Do note that reviewers will not review PRs whose CI builds are failing, or are in a Draft state. Please only submit code for review that comply with the Definition of Done.
We encourage all managers and developers to review any Pull Request that seems interesting to them. When doing so, please leave comments following Conventional Comments. This is required in order to better indicate the comment's intent.
PR Branch Naming¶
- Branches MUST include a prefix with the name of the bounded context. i.e:
datasync/
,service-order/
,position/
. - Branches MUST also include the type of branch:
features/
,fixes/
,docs/
,refactors/
,pocs/
, or others. Do note that the types are plural. -
Branch name SHOULD be kebab-cased. i.e :
datasync/features/my-super-cool-feature
service-order/fixes/super-tickets
PR Title¶
- The Title MUST follow Conventional Commits.
- Optionally you can use as scope the name of the bounded context.
Some examples:
chore(service-order): comments improvements
refactor(datasync): renames SuperClassName to MegaClassName
feat(aims): adds components integration
fix(datasync): fixes #193
PR Template¶
All Pull Requests will get a template pre-assigned on their description, which has to be filled by the creator before creating the PR.
Please try to be concise and explain as best as you can why are you requesting
those changes to be pulled into the main
branch.
Release Strategy¶
The monorepo tells us what to release, not when. We know that the next time we release A, it will be the current HEAD version of A at that time. That may or may not require other services to be released as well.
For the Suite Framework, we will most likely increment all package versions when we release, for consistency with .NET meta-package framework.
When we need to perform a new release, we may create a new release branch if we
need a "clean cut" for QA testing. However, bugfixes that are discovered
must be applied to main
and the cherry-picked to the release. Never merge
from the release branch to main
.
The release branch is just a temporary artifact. main
needs to be tagged in
order to keep a sane release history for bugfixes.
To be able to merge changes to main
more frequently, while also being able to
cleanly cut a release, consider applying
Branching by Abstraction
Definition of Done¶
This applies to Suite Team members. Contributors are encouraged to follow it, but if the time is not there, the PR may be finished by the Suite Team. However, a PR should pass this checklist nevertheless:
- All code MUST be unit tested. Coverage must exceed 80%. The new code coverage percentage MUST be greater or equal than the current coverage percentage. Be creative, test edge cases. See our testing guidelines for further information on this topic.
- Documentation MUST be present for the new feature according to the Suite's documentation standard.
- Samples MUST be present. At the very least for the core functionality of the feature.
- Code MUST be up to the Suite's Coding Standards and the ITsynch Style Guide
- Pull Request MUST be linked to a User Story
- .NET code MUST make proper use of logging tooling as per the Observability Section
Spell checking¶
As part of the CI process, we run a spell checker on all the code. We keep a repository specific dictionary with all terminology that's not found on the builtin English dictionary.
Adding a new word to said dictionary is as simple as adding a new line with said
word into the /scripts/spell-checking/dictionary.txt
file. VSCode auto
suggestions can be used to do so, or the file can be edited manually.
However, before doing so, consider if the word you're adding will be used
multiple times throughout the code base, or if it's a one off term that only
needs to be written once in a single file. If it's the later, then please use a
// cSpell:ignore <YOUR_WORD>
directive on your file (either at the top, or
close to the word you want to ignore). This way, localized exceptions don't risk
masking an actual spelling error somewhere else.
.NET particularities¶
- Localization files for Spanish language will be spell checked against an Spanish dictionary. This is the only case where we won't spell check in english. Just to clarify, Spanish localization files are not required, but we have some, hence the exception.
- Text written inside ORM configuration functions like
ToTable
,Property
,ForeignKey
, etc is exempted from spell checking (in order to avoid adding it to the global dictionary). Additionally, we shouldn't configure table and property names by hand, except when we don't have control over the database schema (i.e. AMOS). Please pay attention to your database mapping files :)