Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move contributor documentation to CONTRIBUTING.md #7381

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
119 changes: 117 additions & 2 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,79 @@ Welcome, and thank you for your interest in contributing to iTwin.js!
There are many ways to contribute.
The goal of this document is to provide a high-level overview of how you can get involved.

- [Contributing to iTwin.js](#contributing-to-itwinjs)
- [Repo Setup](#repo-setup)
- [Source Code Edit Workflow](#source-code-edit-workflow)
- [Other NPM Scripts](#other-npm-scripts)
- [Asking Questions](#asking-questions)
- [Providing Feedback](#providing-feedback)
- [Reporting Issues](#reporting-issues)
- [Look For an Existing Issue](#look-for-an-existing-issue)
- [Writing Good Bug Reports and Feature Requests](#writing-good-bug-reports-and-feature-requests)
- [Follow Your Issue](#follow-your-issue)
- [Contributing Guidelines](#contributing-guidelines)
- [Branch Naming Policy](#branch-naming-policy)
- [Contributor License Agreement (CLA)](#contributor-license-agreement-cla)
- [Pull Requests](#pull-requests)
- [Types of Contributions](#types-of-contributions)
- [Frequently Asked Questions](#frequently-asked-questions)
- [Rush commands take too long, is there a way to speed up my contribution workflow?](#rush-commands-take-too-long-is-there-a-way-to-speed-up-my-contribution-workflow)
- [Do I have to rebuild all packages in the repo, even those I didn't work on?](#do-i-have-to-rebuild-all-packages-in-the-repo-even-those-i-didnt-work-on)
- [A subdirectory can not find a node\_modules file or directory](#a-subdirectory-can-not-find-a-node_modules-file-or-directory)
- [Updating dependencies/devDependencies on packages within the monorepo](#updating-dependenciesdevdependencies-on-packages-within-the-monorepo)
- [Updating dependencies/devDependencies on packages external to monorepo](#updating-dependenciesdevdependencies-on-packages-external-to-monorepo)

## Repo Setup

This repository is a [monorepo](https://monorepo.tools/) that holds the source code for multiple [iTwin.js npm packages](https://www.npmjs.com/search?q=%40itwin). It is built using [Rush](http://rushjs.io/).

Each package has its own **node_modules** directory that contains symbolic links to *common* dependencies managed by Rush.

1. Install dependencies: `rush install`
2. Build source: `rush build`
3. Run tests: `rush cover`

The above commands iterate and perform their action against each package in the monorepo.

## Source Code Edit Workflow

<details>
<summary>Once you've set up the repo, you can move on to making changes to the source code by following the steps within: </summary>

1. Create your own branch by running `git checkout -b "<your-branch-name>"`. See [Branch Naming Policy](#branch-naming-policy)
2. Make source code changes.
3. Rebuild the repo by running `rush build`.
4. Ensure unit tests pass when run locally: `rush cover`.
5. Ensure linting passes when run locally: `rush lint`.
6. Locally commit changes: `git commit` (or use the Visual Studio Code user interface).
7. Repeat steps 1-4 until ready to push changes.
8. Check for API signature changes: `rush extract-api`. This will update the signature files, located in `common/api`. **Note:** before doing this, first do the following:
- Be sure that your branch is up to date with the target branch (i.e. `git merge origin/master`).
- Clean up your build output: `rush clean`.
- Rebuild the project: `rush build`.
9. Review any diffs to the API signature files in the `common/api` directory to ensure they are compatible with the intended release of the package.
- If any differences are in packages not modified on this branch, revert the changes before committing.
10. Add changelog entry (which could potentially cover several commits): `rush change`.
11. Follow prompts to enter a change description or press ENTER if the change does not warrant a changelog entry. If multiple packages have changed, multiple sets of prompts will be presented. If the changes are only to non-published packages (like **display-test-app**), then `rush change` will indicate that a changelog entry is not needed.
12. Completing the `rush change` prompts will cause new changelog entry JSON files to be created.
13. Add and commit the changelog JSON files and any API signature updates.
14. Publish changes on the branch and open a pull request.

> If executing scripts from `package.json` files in any of the subdirectories, we recommend using [`rushx`](https://rushjs.io/pages/commands/rushx/) over `npm`.
tcobbs-bentley marked this conversation as resolved.
Show resolved Hide resolved

If using the command line on Windows, steps 8 through 11 above can be completed in one step by running `rushchange.bat` from the imodeljs root directory.
Only use `rushchange.bat` if none of the changes require a changelog entry.
> The CI build will break if changes are pushed without running `rush change` and `rush extract-api` (if the API was changed). The fix will be to complete steps 6 through 11.

Here is a sample [changelog](https://github.com/microsoft/rushstack/blob/master/apps/rush/CHANGELOG.md) to demonstrate the level of detail expected.

</details>

### Other NPM Scripts

1. Build TypeDoc documentation for all packages: `rush docs`
2. Build TypeDoc documentation for a single package: `cd core\backend` and then `rushx docs`

## Asking Questions

Have a question?
Expand Down Expand Up @@ -63,11 +136,14 @@ They will simply ask for more information!

You may be asked to clarify things or try different approaches, so please follow your issue and be responsive.

## Contributions
## Contributing Guidelines

We'd love to accept your contributions to iTwin.js.
There are just a few guidelines you need to follow.

### Branch Naming Policy

We recommend putting your github username, followed by a succinct branch name that reflects the changes you want to make. Eg. ` git checkout -b "<gh_username>/cleanup-docs"`
### Contributor License Agreement (CLA)

A [Contribution License Agreement with Bentley](https://gist.github.com/imodeljs-admin/9a071844d3a8d420092b5cf360e978ca) must be signed before your contributions will be accepted. Upon opening a pull request, you will be prompted to use [cla-assistant](https://cla-assistant.io/) for a one-time acceptance applicable for all Bentley projects.
Expand All @@ -89,6 +165,45 @@ We welcome contributions, large or small, including:
- Example code snippets
- Sample data

If you would like to contribute new APIs, please check the [CODEOWNERS](https://github.com/iTwin/itwinjs-core/blob/master/.github/CODEOWNERS) file to ensure your contributions align with the package owner's and project goals.
If you would like to contribute new APIs, create a new [issue](https://github.com/iTwin/itwinjs-core/issues) and explain what these new APIs aim to accomplish. If possible check the [CODEOWNERS](https://github.com/iTwin/itwinjs-core/blob/master/.github/CODEOWNERS) file and tag the owners of the package you plan on introducing the new APIs into.

Thank you for taking the time to contribute to open source and making great projects like iTwin.js possible!

## Frequently Asked Questions

### Rush commands take too long, is there a way to speed up my contribution workflow?

If your source code change only impacts the subdirectory you are working on, you can use a package manager to run local commands, without affecting other packages.

Eg. I add a new method within `core/frontend`, also adding a relevant unit test in that folder's `src/test`. I can navigate to the root of that subdirectory, and run `rushx build`, followed by `rushx test` or `rushx cover`.

### Do I have to rebuild all packages in the repo, even those I didn't work on?
No. For incremental builds, the `rush build` command can be used to only build packages that have changes versus `rush rebuild` which always rebuilds all packages.

> It is a good idea to `rush install` after each `git pull` as dependencies may have changed.
>
### A subdirectory can not find a node_modules file or directory
If you get an error similar to the following:

```
[Error: ENOENT: no such file or directory, stat '/.../itwinjs-core/test-apps/display-test-app/node_modules/@bentley/react-scripts']
```
This means that the repo has stopped making use of an npm package that was used in the past:

To fix this build error, you should completely remove the node_modules directory and reinstall your dependencies. `rush update --purge` is a one-line solution for the above.

### Updating dependencies/devDependencies on packages within the monorepo

The version numbers of internal dependencies (see [example](https://github.com/iTwin/itwinjs-core/blob/e17654e4eca60c66bd1888f032aa03ef39d4c8a3/core/bentley/package.json#L3)) should not be manually edited.
These will be automatically updated by our internal CI pipelines.
Note that the packages are published by CI builds only.

### Updating dependencies/devDependencies on packages external to monorepo

Use these instructions to update dependencies and devDependencies on external packages (ones that live outside of this monorepo).

1. Go into the appropriate `package.json` file and update the semantic version range of the dependency you want to update.
2. Run `rush check` to make sure that you are specifying consistent versions across the repository
3. Run `rush update` to make sure the newer version of the module specified in #1 is installed


80 changes: 11 additions & 69 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,10 @@ This repository is a [monorepo](https://en.wikipedia.org/wiki/Monorepo) that hol

See [rush.json](./rush.json) for the complete list of packages and [Versioning.md](./Versioning.md) for package and API versioning policies.

Each package has its own **node_modules** directory that contains symbolic links to *common* dependencies managed by Rush.
## Quick Start

- This is a [sample](https://www.itwinjs.org/sandboxes/iTwinPlatform/3d%20Viewer) of an iTwin viewer - a frontend application that displays infrastructure projects on browsers. It uses many of the APIs and libraries published from this repository.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thoughts on hiding the code, and just showing the viewer?

https://www.itwinjs.org/sandbox/iTwinPlatform/3d%20Viewer?mainHeader=hidden&editorPane=hidden&headers=hidden

go to the ellipses > embed sandbox > mess around with the option & copy the iframe url

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

our users will still be developers - it'll be nice to show them the coding panel - users can play around with the code, and the panel encourages their own discovery. (If the code is outdated, that's a different issue to tackle)

- You can also look at [other samples](https://developer.bentley.com/samples/) which showcases the capabilities of iTwin.js, and the iTwin Platform.
## Prerequisites
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks pretty neat. I made a change to move the source code edit workflow section in CONTRIBUTING.md to use a collapsible block. The prereqs section of the root README isn't too text heavy yet, so I'm thinking we leave that alone for now, and revisit if the README gets too big


- [Git](https://git-scm.com/)
Expand All @@ -39,79 +41,19 @@ Each package has its own **node_modules** directory that contains symbolic links

> See [supported platforms](./docs/learning/SupportedPlatforms.md) for further information.

## Build Instructions

1. Clone repository (first time) with `git clone` or pull updates to the repository (subsequent times) with `git pull`
MichaelSwigerAtBentley marked this conversation as resolved.
Show resolved Hide resolved
2. Install dependencies: `rush install`
3. Clean: `rush clean`
4. Build source: `rush build`
5. Run tests: `rush cover`

The `-v` option for `rush` is short for `--verbose` which results in a more verbose command.

The above commands iterate and perform their action against each package in the monorepo.

If you get an error similar to the following, it means that the repo has stopped making use of an npm package that was used in the past.

```
[Error: ENOENT: no such file or directory, stat '/.../itwinjs-core/test-apps/display-test-app/node_modules/@bentley/react-scripts']
```

To fix this build error, you should completely remove the node_modules directory that is referenced in the error with `rush purge`, and then rerun `rush install`.

For incremental builds, the `rush build` command can be used to only build packages that have changes versus `rush rebuild` which always rebuilds all packages.

> Note: It is a good idea to `rush install` after each `git pull` as dependencies may have changed.

## Source Code Edit Workflow

1. Make source code changes on a new Git branch
2. Ensure unit tests pass when run locally: `rush cover`
3. Ensure linting passes when run locally: `rush lint`
4. Locally commit changes: `git commit` (or use the Visual Studio Code user interface)
5. Repeat steps 1-4 until ready to push changes
6. Check for API signature changes: `rush extract-api`. This will update the signature files, located in `common/api`. **Note:** before doing this, first do the following:
- Be sure that your branch is up to date with the target branch (i.e. `git merge origin/master`)
- Cleanup your build output: `rush clean`
- Rebuild the project: `rush build`
7. Review any diffs to the API signature files in the `common/api` directory to ensure they are compatible with the intended release of the package.
- If any differences are in packages not modified on this branch, revert the changes before committing.
8. Add changelog entry (which could potentially cover several commits): `rush change`
9. Follow prompts to enter a change description or press ENTER if the change does not warrant a changelog entry. If multiple packages have changed, multiple sets of prompts will be presented. If the changes are only to non-published packages (like **display-test-app**), then `rush change` will indicate that a changelog entry is not needed.
10. Completing the `rush change` prompts will cause new changelog entry JSON files to be created.
11. Add and commit the changelog JSON files and any API signature updates.
12. Publish changes on the branch and open a pull request.

> If executing scripts from `package.json` files in any of the subdirectories, we recommend using [`rushx`](https://rushjs.io/pages/commands/rushx/) over `npm`.

If using the command line, steps 8 through 11 above can be completed in one step by running `rushchange.bat` from the imodeljs root directory.
Only use `rushchange.bat` if none of the changes require a changelog entry.
> Note: The CI build will break if changes are pushed without running `rush change` and `rush extract-api` (if the API was changed). The fix will be to complete steps 6 through 11.

Here is a sample [changelog](https://github.com/microsoft/rushstack/blob/master/apps/rush/CHANGELOG.md) to demonstrate the level of detail expected.

## Updating dependencies/devDependencies on packages within the monorepo

The version numbers of internal dependencies should not be manually edited.
These will be automatically updated by the overall *version bump* workflow.
Note that the packages are published by CI builds only.

## Updating dependencies/devDependencies on packages external to monorepo

Use these instructions to update dependencies and devDependencies on external packages (ones that live outside of this monorepo).

1. Edit the appropriate `package.json` file to update the semantic version range
2. Run `rush check` to make sure that you are specifying consistent versions across the repository
3. Run `rush update` to make sure the newer version of the module specified in #1 is installed
## Contribution

MichaelSwigerAtBentley marked this conversation as resolved.
Show resolved Hide resolved
## Other NPM Scripts
### Developer Quick Start

1. Build TypeDoc documentation for all packages: `rush docs`
2. Build TypeDoc documentation for a single package: `cd core\backend` and then `rushx docs`
The following instructions will quickly set the repo up for you to edit the source code and contribute:

## Contribution
1. Clone the repository locally: `git clone https://github.com/iTwin/itwinjs-core.git`
2. Install dependencies: `rush install`
3. Build source: `rush build`
4. Run tests: `rush cover`

If you have questions, or wish to contribute to iTwin.js, see our [Contributing guide](./CONTRIBUTING.md).
For more information, our [Contributing guide](./CONTRIBUTING.md) contains detailed instructions on typical source code editing workflows, our contribution standards, FAQs, instructions on how to post questions and et cetera.

## Licensing

Expand Down
Loading