From be3bed41eb973ba9bdc69c69d168ef7a13d17ccf Mon Sep 17 00:00:00 2001 From: Suzanne Rozier Date: Fri, 26 Mar 2021 15:32:49 -0500 Subject: [PATCH] docs: Add guidance for PR titles and testing in an application (#1028) * Add docs with more info on commit message guidelines * Add section about application testing to docs * Fix links * Minor updates to ordering --- .github/pull_request_template.md | 2 + README.md | 13 ++- docs/contributing.md | 169 ++++++++++++++++++++++++++----- package.json | 18 +++- 4 files changed, 172 insertions(+), 30 deletions(-) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 6b84a0d3cc..3ffcdfe045 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -8,4 +8,6 @@ + + ### Screenshots (optional) diff --git a/README.md b/README.md index f6613e80bb..6827fe8866 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,9 @@ # @trussworks/react-uswds + + [![All Contributors](https://img.shields.io/badge/all_contributors-13-orange.svg?style=flat-square)](#contributors-) + [![npm version](https://img.shields.io/npm/v/@trussworks/react-uswds)](https://www.npmjs.com/package/@trussworks/react-uswds) @@ -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) @@ -162,4 +173,4 @@ Thanks goes to these wonderful people ([emoji key](https://allcontributors.org/d -This project follows the [all-contributors](https://github.com/all-contributors/all-contributors) specification. Contributions of any kind welcome! \ No newline at end of file +This project follows the [all-contributors](https://github.com/all-contributors/all-contributors) specification. Contributions of any kind welcome! diff --git a/docs/contributing.md b/docs/contributing.md index 2fe096a735..9a170d5e8f 100644 --- a/docs/contributing.md +++ b/docs/contributing.md @@ -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) @@ -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. @@ -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. @@ -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. @@ -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: + +``` +(scope): + + + +