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

docs: Add guidance for PR titles and testing in an application #1028

Merged
merged 4 commits into from
Mar 26, 2021
Merged
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
2 changes: 2 additions & 0 deletions .github/pull_request_template.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,6 @@

<!-- Describe how a reviewer could test or verify your changes. -->

<!-- Does this change fix an issue or bug in an application you work on? Make sure you've tested this branch in your application to verify it works before merging & releasing. -->

### Screenshots (optional)
13 changes: 12 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
# @trussworks/react-uswds

<!-- ALL-CONTRIBUTORS-BADGE:START - Do not remove or modify this section -->

[![All Contributors](https://img.shields.io/badge/all_contributors-13-orange.svg?style=flat-square)](#contributors-)

<!-- ALL-CONTRIBUTORS-BADGE:END -->

[![npm version](https://img.shields.io/npm/v/@trussworks/react-uswds)](https://www.npmjs.com/package/@trussworks/react-uswds)
Expand Down Expand Up @@ -114,6 +117,14 @@ Are you a Trussel and new to this project? Check out our [on & offboarding guide

This repository is governed by the [Contributor Covenant](./CODE_OF_CONDUCT.md)

### Quick links:

- [Yarn scripts](./docs/contributing#available-commands)
- [PR commit guidelines](./docs/contributing.md#opening--merging-pull-requests)
- [Adding new components](./docs/adding_new_components.md)
- [Testing in an application](./docs/contributing.md#testing-in-an-application)
- [Releasing](./docs/releasing.md)

## License

[Copyright 2021, TrussWorks, Inc.](./LICENSE)
Expand Down Expand Up @@ -162,4 +173,4 @@ Thanks goes to these wonderful people ([emoji key](https://allcontributors.org/d

<!-- ALL-CONTRIBUTORS-LIST:END -->

This project follows the [all-contributors](https://github.com/all-contributors/all-contributors) specification. Contributions of any kind welcome!
This project follows the [all-contributors](https://github.com/all-contributors/all-contributors) specification. Contributions of any kind welcome!
169 changes: 145 additions & 24 deletions docs/contributing.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,20 @@

We are glad you are interested to contribute to this project! Please make sure you've seen the [README](../README.md) and [license](../README.md#license) for this project before continuing further. If you work for Truss, check out the [guide for Trussels](./for_trussels.md) as well.

We welcome contributions in the form of comments, issues, or pull requests with code changes. If you see an error, have a question, or want to share feedback open an issue to discuss. This repository is governed by the [Contributor Covenant](../CODE_OF_CONDUCT.md)
We welcome contributions in the form of comments, issues, or pull requests with code changes. If you see an error, have a question, or want to share feedback open an issue to discuss. This repository is governed by the [Contributor Covenant](../CODE_OF_CONDUCT.md).

**Table of Contents**

- [Contributing](#contributing)
- [Environment Setup](#environment-setup)
- [Available Commands](#available-commands)
- [Development](#development)
- [Before You Start](#before-you-start)
- [Pull Requests](#pull-requests)
- [Guidelines](#guidelines)
- [Linting, Testing, and Automation](#linting-testing-and-automation)
- [Environment setup](#environment-setup)
- [Available commands](#available-commands)
- [Development](#development)
- [Working on an issue](#working-on-an-issue)
- [General guidelines](#general-guidelines)
- [Linting, formatting, & automated tests](#linting-formatting--automated-tests)
- [Testing in an application](#testing-in-an-application)
- [Opening & merging pull requests](#opening--merging-pull-requests)

## Environment Setup
## Environment setup

1. Use the node environment manager of your choice, but make sure you have the required version specified in `.node-version`. We recommend using [nodenv](https://github.com/nodenv/nodenv) to manage your node versions, but you can also use [homebrew](https://brew.sh/). More info can be found here: [how to install Node.js](https://nodejs.dev/how-to-install-nodejs)

Expand All @@ -26,7 +26,7 @@ We welcome contributions in the form of comments, issues, or pull requests with

3. Clone this repo and make sure you can run all of the available commands listed below with no errors.

### Available Commands
### Available commands

These should all be run from within the project directory.

Expand All @@ -48,25 +48,19 @@ These should all be run from within the project directory.

## Development

### Before You Start

Make sure you understand the [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/#summary) specification. **All pull requests opened into `main` must have a title that follows the [conventional commits spec](https://github.com/conventional-changelog/commitlint/tree/master/%40commitlint/config-conventional)** This generates an automated changelog entry and is later used to determine versioning for release. It is required to merge.

### Pull Requests
### Working on an issue

To begin working on an issue, make sure you've assigned yourself to the issue in Github and marked it as "In Progress.". If there isn't an issue yet for what you want to work on, [please create one](https://github.com/trussworks/react-uswds/issues/new/choose).

Create a new branch off `main` using the naming convention:

`{your initials or username}-{summary}-{issue #}`
Some issues are tagged with the [needs requirements](https://github.com/trussworks/react-uswds/issues?q=is%3Aissue+is%3Aopen+label%3A%22status%3A+needs+requirements%22) label, which usually indicates there is some ongoing discussion about the issue and it may not be ready to start development on yet.

For example: `hw-accordion-component-112`, `hw-accordion-component-112`
Once you have an issue to work on, create a new branch off `main` using the naming convention:

When your branch is ready for review, open a PR into `main` and request reviews from relevant team members. Reviews from codeowners will automatically be requested. Address any failing tests, lint errors, PR feedback, etc. Once the work is approved, it will be merged with **squash & merge**.
`{your initials or username}-{summary}-{issue #}`

> Note: Currently our CI cannot run directly on external PRs (work from outside the Truss organization) and prevents merge. To manage this, we pull these PRs into a separate branch that a CODEOWNER creates, run automation, and merge from there. Your initial PR will be closed with a comment and your work will be merged instead from the related PR.
For example: `hw-accordion-component-112`

### Guidelines
### General guidelines

- Encourage a strict separation of concerns, focusing on UI (rendered HTML and CSS) rather than any application logic.
- Expose the necessary props for composability and extensibility, such as event handlers, custom CSS classes, etc.
Expand All @@ -76,7 +70,7 @@ When your branch is ready for review, open a PR into `main` and request reviews

More guidance for preferred React practices can be found in the [adding new components](./adding_new_components.md) documentation.

### Linting, Testing, and Automation
### Linting, formatting, & automated tests

Because this project exports a library that will be used by other projects, it is important that updates follow a set of standard practices. When you commit your changes, several hooks will run to check and format staged files. In order to be eligible for merging, all branches must pass the following automation.

Expand All @@ -102,3 +96,130 @@ Because this project exports a library that will be used by other projects, it i
- The **[WIP]** prefix can be used to indicate a pull request is still work in progress. In this case, the PR title is not validated and the pull request lint check remains pending.

Having issues? See [FAQs](./faqs.md).

### Testing in an application

It's important to test your changes in a real-life application instance, especially if you're working on a bugfix or issue that is specific to the needs of an application. There are a few ways to do this.

#### `yarn link`

Yarn provides the [`yarn link` command](https://classic.yarnpkg.com/en/docs/cli/link/) to symlink a specific package to a local version of that package. This can be very helpful if you're trying to do development in ReactUSWDS & test changes in an application simultaneously.

To use this, first run `yarn link` in your local ReactUSWDS directory:

```
cd /path/to/react-uswds
➜ yarn link
yarn link v1.22.4
success Registered "@trussworks/react-uswds".
info You can now run `yarn link "@trussworks/react-uswds"` in the projects where you want to use this package and it will be used instead.
```

Then, link the package in your application directory:

```
cd /path/to/your/application
➜ yarn link @trussworks/react-uswds
yarn link v1.22.4
success Using linked package for "@trussworks/react-uswds".
```

You can then run `yarn build:watch` in ReactUSWDS and your application's compiler at the same time, and when you save code changes in ReactUSWDS it will trigger a build of the package, which will then trigger a build of your application's code.

> **Warning:** Make sure to **unlink** the package once you're done making changes! It can be easy to forget you're using a linked package in your application, and commit changes without switching back to a released version of ReactUSWDS.

To unlink the package from your application code (you don't need to unlink your local ReactUSWDS code)
:

```
cd /path/to/your/application
➜ yarn unlink @trussworks/react-uswds
yarn unlink v1.22.4
success Removed linked package "@trussworks/react-uswds".
info You will need to run `yarn install --force` to re-install the package that was linked.

➜ yarn install --force
```

#### Install from a ReactUSWDS branch

Another option is to install a specific branch of ReactUSWDS to your application that hasn't been released yet. To do that, you will need to change the reference in your application's `package.json` file:

```
"@trussworks/react-uswds": "https://github.com/trussworks/react-uswds#name-of-branch",
```

After making that change, run `yarn install` to install that version of the package. This will also build the ReactUSWDS code so it can be used in your application as if it were being installed from NPM. You can verify the source of the package by looking in your application's `yarn.lock` file:

```
"@trussworks/react-uswds@https://github.com/trussworks/react-uswds":
version "1.13.2"
resolved "https://github.com/trussworks/react-uswds#92ea5f07f7212370165423ec09ed35eec3aa7e58"
```

The important thing to note here is that `resolved` is pointing to the Github repo instead of the package registry, and the SHA should match the last commit of the branch you're using.

The downside of this method is that if you need to make changes to ReactUSWDS, you will have to commit and push up the branch, and then run `yarn upgrade @trussworks/react-uswds` to install the newest changes in your application. If you're ever not sure if your application code is referencing the right version, you can always verify against the SHA shown in the `yarn.lock` file.

You can commit this change to your application code if you want to use a branch of ReactUSWDS for some time, but make sure to switch back to a released version once the branch is merged and released!

### Opening & merging pull requests

When your branch is ready for review, open a new pull request into `main` and request reviews from relevant team members. Reviews from codeowners will automatically be requested. Address any failing tests, lint errors, PR feedback, etc. Once the work is approved, it will be merged with **squash & merge**.

When opening the pull request, it's important to understand the [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/#summary) specification. **All pull requests opened into `main` must have a title that follows the [conventional commits spec](https://github.com/conventional-changelog/commitlint/tree/master/%40commitlint/config-conventional)**. This generates an automated changelog entry and is later used to determine versioning for release. It is required to merge.

> Note: Currently our CI cannot run directly on external PRs (work from outside the Truss organization) and prevents merge. To manage this, we pull these PRs into a separate branch that a CODEOWNER creates, run automation, and merge from there. Your initial PR will be closed with a comment and your work will be merged instead from the related PR.

The format for PR commits is:

```
<type>(scope): <subject>
<BLANK LINE>
<body>
<BLANK LINE>
<footer>
```

#### `type`:

Required, must be one of the following:

- `build`: Changes that affect the build system or external dependencies (example scopes: webpack, npm)
- `chore`: Completing a task that has no effective code changes, such as updating the version and changelog for a release
- `ci`: Changes to our CI configuration files and scripts (example scopes: Circle, Github actions/workflows)
- `docs`: Documentation only changes
- `feat`: A new feature
- `fix`: A bug fix
- `perf`: A code change that improves performance
- `refactor`: A code change that neither fixes a bug nor adds a feature
- `revert`: If the commit reverts a previous commit, it should begin with `revert:` , followed by the header of the reverted commit. In the body it should say: `This reverts commit <hash>.`, where the hash is the SHA of the commit being reverted.
- `style`: Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc)
- `test`: Adding missing tests or correcting existing tests

#### `scope`:

Optional, can be one of the following:

- `deps`: Updating a package listed in dependencies
- `deps-dev`: Updating a package listed in devDependencies
- `release`: Releasing a new version
- `circleci`: Changes to CircleCI config and/or scripts
- `example`: Changes to the Example App only
- `storybook`: Changes to Storybook or stories files only

#### `body`:

Will default to a list of included commits since PRs are all squashed when merging. You can also add additional context to the body if needed.

#### `footer`:

Should link the PR to any issues it closes (see: ["Linking a pull request to an issue"](https://docs.github.com/en/github/managing-your-work-on-github/linking-a-pull-request-to-an-issue)).

Example:

```
Closes #123
```

Should also include `BREAKING CHANGE:` if the commit includes any breaking changes. This will make sure the major version is automatically bumped when this commit is released.
18 changes: 13 additions & 5 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,18 @@
"tag": true
},
"types": [
{
"type": "build",
"hidden": true
},
{
"type": "chore",
"hidden": true
},
{
"type": "ci",
"hidden": true
},
{
"type": "feat",
"section": "Features"
Expand All @@ -164,10 +176,6 @@
"type": "fix",
"section": "Bug Fixes"
},
{
"type": "chore",
"hidden": true
},
{
"type": "docs",
"section": "Documentation & Examples"
Expand All @@ -182,7 +190,7 @@
},
{
"type": "perf",
"hidden": true
"section": "Performance Improvements"
},
{
"type": "test",
Expand Down