Skip to content

Latest commit

 

History

History
104 lines (72 loc) · 6.06 KB

CONTRIBUTING.md

File metadata and controls

104 lines (72 loc) · 6.06 KB
Raw

Guidelines for contributing to Composer

  • Nothing gets pushed to the main branch. Use github pull-request's to submit new code.
  • Use the "Description" field of the new Pull Request form to describe the change in detail. Assume reviewers are taking on a context switch to review your code and need a refresher on context. Reference & link to any pertinent tickets in project tracking software. This will also act as a paper trail of changes and it should be logical for one to use git blame to look up the commit and understand why the change was made.
  • Keep your pull-requests small and concise. If you are working on a substantially large change in lines of code, it is OK to open a pull-request that is a partial implementation; please make it clear in the description and keep the boundaries between diffs logical.
  • If the change includes non-trivial changes in UI, include a screenshot and tag a designer on the project. Be sure to show any border or layout changes clearly. Use your best judgement if you would like a designer to explicitly sign off on it, such as a deviation from the design artifacts.
  • If the change includes a non-trivial change in UX, include a gif demonstrating the interaction. Tag a designer and use your judgement if you would like explicit sign off from them on it.
  • At least one approver is required on a pull-request prior to merging it.
  • Programatic testing for each change is a requirement. Comprehensive unit tests for every change is not. "Write tests, not too many, mostly integration". We should be using Jest & @testing-library/react for this in the client. Code coverage benchmarks will be introduced and need to be met with each change (TBD).
  • Write code with Internationalization & Accessibility in mind. Every rendered string should be wrapped in an i18n API. Scrub through each UI change for keyboard-navigation and focus-indication. This will prevent most accessibility bugs.
  • Use rebase when merging changes into to the main branch. This can be done using the “Squash and Merge” technique in the GitHub UI. Local branches will need to be updated using rebase as well. This will keep a clean commit history. Reach out to me if you need help understanding rebase.
  • Features pushed behind a feature flag (regardless if the feature flag is hidden or not) need to follow all traditional PR requirements to the main branch.

Forking

In order to keep a clean tree on our Github repository we use a forking strategy. Follow the steps below to work with forks.

Creating a fork

To create a fork, click on the the Fork button at the top of the repository page in Github.

This creates a copy of the repository in your Github account in which you can develop against. Read more about working with forks here.

Developing on your fork

  • clone your fork to your machine
  • create a branch
  • make changes and commit
  • push your branch to your fork
  • create a pull request against the main repository docs

Keeping your fork up-to-date

  • add a new remote to your local repository
git remote add upstream git@github.com:microsoft/BotFramework-Composer.git
  • update your upstream remote
  • merge upstream changes into your local main branch
git fetch upstream
git checkout main
git pull upstream main

Updating your feature branch with latest main

  • update your local main by following the previous section
  • merge/rebase main into your feature branch
git checkout <feature-branch-name>
git merge main # git rebase main
git push origin <feature-branch-name>

Checking out a pull request from a fork

git checkout -b pull-request-branch main
git pull git://github.com/<username>/BotFramework-Composer.git <fork-branch-name>

For example, if there were a PR from the user AwesomeDev's branch my-awesome-feature:

git checkout -b AwesomeDev/my-awesome-feature main
git pull git://github.com/AwesomeDev/BotFramework-Composer.git my-awesome-feature

Things to keep in mind

  • Always work off of branches; don't commit directly to your main branch. This will help avoid conflicts and keep your main branch pristine.
  • When creating pull requests check the Allow edits from maintainers option so that others can make changes if necessary.

Testing

There are two types of tests in the Composer project: unit tests and end-to-end (e2e) tests. Use e2e tests to cover core scenarios (happy path) with some coverage of edge cases. Everything else should be unit tested, with a target coverage goal of 90%. The primary outcome of a well-tested code base is greater confidence in making future changes.

Unit Testing

Jest is the unit testing framework used. The guiding principle to unit testing is to test the behavior of the code, not the mechanics.

  • When testing UI, make assertions about the state of the DOM. Don't let component implementation details leak into tests.
  • It's ok to mock/stub side effecting code (xhr requests, file system reads/writes, etc).
  • Each test should be independent (isolated) from other tests. Make sure to reset state, clean up either before or after eact test run.
  • Provide a concise description of what tests cover using the doc string.

Goal: 90% test coverage.

E2E Testing

Cypress is the e2e testing framework. The main guiding principles for e2e tests boil down to the following:

  • E2E tests should be reliable. There should be a high level of confidence in the tests to catch real issues, avoiding flaky tests or false negatives.
  • Work within the framework to make tests work, avoiding using cy.wait as that often is a sign of a poorly written test.
  • It's ok to automate environment setup to reduce timing and false negatives.
  • Use data-testid to make assertions instead of strings. This helps reduce churn when copy updates occur.

Goal: All P0 scenarios have e2e test coverage. Some P1 scenarios have e2e coverage.