Skip to content

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:

  1. Suite Framework: Leonardo Chaia (lchaia@itsynch.com)
    1. framework
    2. modules
    3. runtime
    4. samples
    5. templates
    6. tools
    7. Suite Framework and its Modules documentation.
  2. DataSync: Alan Asmis (aasmis@itsynch.com)
    1. DataSync
    2. Datasync and its Modules documentation.
  3. Amos: Lucas Kloster (lkloster@itsynch.com)
    1. All Amos related .NET services
  4. Users: Lucas Kloster (lkloster@itsynch.com)
    1. All Users related .NET services
  5. Positions: Lucas Kloster (lkloster@itsynch.com)
    1. All Positions related .NET services
  6. Departments: Lucas Kloster (lkloster@itsynch.com)
    1. All Departments related .NET services
  7. UxRoles: Lucas Kloster (lkloster@itsynch.com)
    1. All UxRoles related .NET services
  8. GeneralArrangements: Lucas Kloster (lkloster@itsynch.com)
    1. All GeneralArrangements related .NET services
  9. Scheduler: Lucas Kloster (lkloster@itsynch.com)
    1. All Scheduler related .NET services
  10. Components: Lucas Kloster (lkloster@itsynch.com)
    1. All Components related .NET services
  11. ServiceOrders: Franco Leonardi (fleonardi@itsynch.com)
    1. ServiceOrders
    2. ServiceOrders modules and their documentation
  12. Inspections: Adhemar Bouchet (abouchet@itsynch.com)
    1. Inspections
    2. Inspections modules and their documentation

Tradeoffs

  1. Suite Framework and services can evolve together at the same peace.
  2. All projects use the same dependency versions for third party libraries. For .NET read this, for Angular, it's the root package.json.
  3. All projects directly reference other in-house projects.
  4. Breaking changes are applied to all projects by the dev performing the change.
    1. Help might be requested from other teams to perform the changes.
    2. 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

  1. Branches MUST include a prefix with the name of the bounded context. i.e: datasync/, service-order/, position/.
  2. Branches MUST also include the type of branch: features/, fixes/, docs/, refactors/, pocs/, or others. Do note that the types are plural.
  3. Branch name SHOULD be kebab-cased. i.e :

    1. datasync/features/my-super-cool-feature
    2. service-order/fixes/super-tickets

PR Title

  1. The Title MUST follow Conventional Commits.
  2. Optionally you can use as scope the name of the bounded context.

Some examples:

  1. chore(service-order): comments improvements
  2. refactor(datasync): renames SuperClassName to MegaClassName
  3. feat(aims): adds components integration
  4. 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:

  1. 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.
  2. Documentation MUST be present for the new feature according to the Suite's documentation standard.
  3. Samples MUST be present. At the very least for the core functionality of the feature.
  4. Code MUST be up to the Suite's Coding Standards and the ITsynch Style Guide
  5. Pull Request MUST be linked to a User Story
  6. .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

  1. 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.
  2. 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 :)