This is the multi-page printable view of this section. Click here to print.
This chapter is about the process of integrating changes into the common code base. If you need help at any stage, open an issue or message us.
OSRD application is split in multiple services written in several languages. We try to follow general code best practices and follow each language specificities when required.
README.md).Consult back-end conventions ‣
Python code is used for some packages and integration testing.
diesel::*./// doc comments belong above #[derive(Trait)] invocations.// comments should generally go above the line in question, rather than in-line.We use ReactJS and all files must be written in Typescript.
The code is linted with eslint, and formatted with prettier.
The applications (osrd eex, osrd stdcm, infra editor, rolling-stock editor) offer views (project management, study management, etc.) linked to modules (project, study, etc.) which contain the components.
These views are made up of components and sub-components all derived from the modules. In addition to containing the views files for the applications, they may also contain a scripts directory which offers scripts related to these views. The views determine the logic and access to the store.
Modules are collections of components attached to an object (a scenario, a rolling stock, a TrainSchedule). They contain :
An assets directory (containing images and other files).
Last but not least, a common directory offering :
The following conventions are generally used in the code:
WARNING: in CSS/React, the scope of a class does not depend on where the file is imported, but is valid for the entire application. If you import an
scssfile in the depths of a component (which we strongly advise against), its classes will be available to the whole application and may therefore cause side effects.
It is therefore highly recommended to be able to easily follow the tree structure of applications, views, modules and components also within the SCSS code, and in particular to nest class names to avoid edge effects.
Some additional conventions:
The store allows you to store data that will be accessible anywhere in the application. It is divided into slices, which correspond to our applications.
However, we must be careful that our store does not become a catch-all. Adding new properties to the store must be justified by the fact that the data in question is “high-level” and will need to be accessible from locations far from each other, and that a simple state or context variable is not appropriate for storing this information.
For more details about redux, please refer to the official documentation.
We use Redux ToolKit to make our calls to the backend.
RTK allows you to make API calls and cache responses. For more details, please refer to the official documentation.
The backend endpoints and types are generated directly from the editoast open API and stored in generatedOsrdApi (using the npm run generate-types command). This generated file is then enriched with other endpoints or types in osrdEditoastApi, which can be used in the application. Note that it is possible to transform mutations into queries when generating generatedOsrdApi by modifying the openapi-editoast-config file.
When an endpoint call must be skipped because a variable is not defined, RTK’s skipToken is used to avoid having to use casts.
Application translation is performed on Weblate.
It is recommended that you use the full path for each import, unless your import file is in the same directory.
ESLint is setup to automatically sort imports in four import groups, each of them separated by an empty line and sorted in alphabetical order :
Regarding type imports/exports, ESLint and Typescript are configured to automatically add type before a type import, which allows to:
type keyword)
If you are not used to Git, follow this tutorial
Create a branch
If you intend to contribute regularly, you can request access to the main repository. Otherwise, create a fork.
Add changes to your branch
Before you start working, try to split your work into macroscopic steps.
At the end of each stop, save your changes into a commit.
Try to make commits of logical and atomic units.
Try to follow style conventions.
Keep your branch up-to-date
git switch <your_branch>
git fetch
git rebase origin/dev
The overall format for git commits is as follows:
component1, component2: imperative description of the change
Detailed or technical description of the change and what motivates it,
if it is not entirely obvious from the title.
: in case of hierarchical relationships, with , otherwise-, _ or . if necessary# is forbidden)Ideally:
To be avoided entirely:
component: update ./some/file.ext: specify the update itself rather than the file, the files
are technical elements welcome in the body of the commitcomponent: fix #42: specify the problem fixed in the title, links (to issue, etc.) are very
welcome in commit’s bodywip: describe the work (and finish it)Welcome to ease review, but do not merge:
fixup! previous commit: an autosquash must be run before the mergeRevert "previous commit of the same PR": both commits must be dropped before mergingAll of OSRD’s projects use the DCO (Developer Certificate of Origin) to address legal matters. The DCO helps confirm that you have the rights to the code you contribute. For more on the history and purpose of the DCO, you can read The Developer Certificate of Origin by Roscoe A. Bartlett.
To comply with the DCO, all commits must include a Signed-off-by line.
To sign off a commit, simply add the -s flags to your git commit command,
like so:
git commit -s -m "Your commit message"
This also applies when using the git revert command.
Now, go in Files -> Preferences -> Settings, search for and activate
the Always Sign Off setting.
Finally, when you’ll commit your changes via the VS Code interface, your commits will automatically be signed-off.
The author of a pull request (PR) is responsible for its “life cycle”. He is responsible for contacting the various parties involved, following the review, responding to comments and correcting the code following review (you could also check dedicated page about code review).
Open a pull request
Once your changes are ready, you have to request integration with the dev branch.
If possible:
area:<affected_area> to show which part of the application have been impacted. It can be done through the web interface.Take feedback into account
Once your PR is open, other contributors can review your changes:
The best case is to avoid large PR and split it in multiple PR1:
- ease the reviewing process and might accelerate it (easier to find an hour to review than half a day)
- is more agile, you will get feedback on the early iteration before proposing the next series of modifications,
- keep the git history cleaner (in case of a
git bisectlooking for a regression for example).In the case where you cannot avoid a large PR, don’t hesitate to ask several reviewers to organize themselves, or even to carry out the review together, reviewers and author.
For large PRs that are bound to evolve over time, keeping corrections during review in separate commits helps reviewers. In the case of multiple reviews by the same person, this can save full re-review (ask for help if necessary):
- Add fixup, amend, squash or reword commits using the git commit documentation.
- Automatically merge corrections into the original commits of your PR and check the result, using
git rebase -i --autosquash origin/dev(just before the merge and once review process is complete).- Push your changes with
git push --force-with-leasebecause you are not just pushing new commits, you are pushing changes to existing commits.
A code review is an iterative process. For a smooth review, it is imperative to correctly configure your github notifications.
It is advisable to configure OSRD repositories as “Participating and @mentions”. This allows you to be notified of activities only on issues and PRs in which you participate.
Maintainers are automatically notified by the
CODEOWNERSsystem. The author of a PR is responsible for advancing their PR through the review process and manually requesting maintainer feedback if necessary.
sequenceDiagram
actor A as PR author
actor R as Reviewer/Maintainer
A->>R: Asks for a review, notifying some people
R->>A: Answers yes or no
loop Loop between author and reviewer
R-->>A: Comments, asks for changes
A-->>R: Answers to comments or requested changes
A-->>R: Makes necessary changes in dedicated "fixups"
R-->>A: Reviews, tests changes, and comments again
R-->>A: Resolves requested changes/conversations if ok
end
A->>R: Rebase and apply fixups
R->>A: Checks commits history
R->>A: Approves the PR
Note right of A: Add to the merge queueFinally continue towards tests ‣
if you are not convinced, look for “Stacked Diff” on the web for more literature on the topic, like Stacked Diffs vs. Trunk Based Development ↩︎
/tests folder.openapi.yaml files must have an integration test.The functional writing of the tests is carried out with the Product Owners, and the developers choose a technical implementation that precisely meets the needs expressed and fits in with the recommendations presented here.
We use Playwright to write end-to-end tests, and vitest to write unit tests.
The browsers tested are currently Firefox and Chromium.
id.The data tested must be public data.
The data required (infrastructure and rolling stock) for the tests are offered in the application’s json files, injected at the start of each test and deleted at the end, regardless of its result or how it is stopped, including with CTRL+C.
This is done by API calls in typescript before launching the actual test.
The data tested is the same, both locally and via continuous integration.
E2E tests are implemented iteratively and delivered alongside feature developments. Note that:
Each test must be atomic: it is self-sufficient and cannot be divided.
A test will target a single feature or component, provided it is not too large. A test will not test an entire module or application; it will necessarily be a set of tests, in order to preserve test atomicity.
If a test needs elements to be created or added, these operations must be carried out by API calls in typescript upstream of the test, as is done for adding data. These elements must be deleted at the end of the test, regardless of the result or how it is stopped, including by CTRL+C.
This allows tests to be parallelized.
However, in certain cases where it is relevant, a test may contain several clearly explained and justified test subdivisions (several test() in a single describe()).
The requirement: “We want to test the addition of a train to a timetable”.
NB: the test will not test all the possibilities offered by the addition of trains; this should be a specific test which would test the response of the interface for all scenarios without adding trains.