From cd84354b401e9c51f06f06ab5dcb202461bae95f Mon Sep 17 00:00:00 2001 From: Hsing-Hui Hsu Date: Wed, 17 Feb 2021 11:16:49 -0800 Subject: [PATCH] docs: fix typos in CONTRIBUTING.md --- CONTRIBUTING.md | 64 ++++++++++++++++++++++++------------------------- 1 file changed, 31 insertions(+), 33 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 8f26e8198abc2..2b1a950a551fe 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -129,12 +129,12 @@ and instead only depend on having a working Docker install. ### Step 1: Open Issue If there isn't one already, open an issue describing what you intend to contribute. It's useful to communicate in -advance, because sometimes, someone is already working in this space, so maybe it's worth collaborating with them -instead of duplicating the efforts. +advance because if someone is already working in this space, it may be worth collaborating with them +instead of duplicating the effort. ### Step 2: Design (optional) -In some cases, it is useful to seek for feedback by iterating on a design document. This is useful +In some cases, it is useful to seek feedback by iterating on a design document. This is useful when you plan a big change or feature, or you want advice on what would be the best path forward. Sometimes, the GitHub issue is sufficient for such discussions, and can be sufficient to get @@ -166,7 +166,7 @@ Work your magic. Here are some guidelines: * Every change requires a unit test * If you change APIs, make sure to update the module's README file * Try to maintain a single feature/bugfix per pull request. It's okay to introduce a little bit of housekeeping - changes along the way, but try to avoid conflating multiple features. Eventually all these are going to go into a + changes along the way, but try to avoid conflating multiple features. Eventually, all these are going to go into a single commit, so you can use that to frame your scope. * If your change introduces a new construct, take a look at the our [example Construct Library](packages/@aws-cdk/example-construct-library) for an explanation of the common patterns we use. @@ -249,7 +249,7 @@ BREAKING CHANGE: Description of what broke and how to achieve this behavior now ### Step 5: Pull Request -* Push to a GitHub fork or to a branch (naming convention: `/`) +* Push to a GitHub fork or to a branch (naming convention: `/`). * Submit a Pull Request on GitHub. A reviewer will later be assigned by the maintainers. * Please follow the PR checklist written below. We trust our contributors to self-check, and this helps that process! * Discuss review comments and iterate until you get at least one “Approve”. When iterating, push new commits to the @@ -262,15 +262,15 @@ BREAKING CHANGE: Description of what broke and how to achieve this behavior now ### Step 6: Merge -* Make sure your PR builds successfully (we have CodeBuild setup to automatically build all PRs) +* Make sure your PR builds successfully (we have CodeBuild setup to automatically build all PRs). * Once approved and tested, a maintainer will squash-merge to master and will use your PR title/description as the commit message. ## Tools -The CDK is a big project, and, at the moment, all of the CDK modules are mastered in a single monolithic repository +The CDK is a big project, and at the moment, all of the CDK modules are mastered in a single monolithic repository (uses [lerna](https://github.com/lerna/lerna)). There are pros and cons to this approach, and it's especially valuable -to maintain integrity in the early stage of the project where things constantly change across the stack. In the future +to maintain integrity in the early stage of the project where things constantly change across the stack. In the future, we believe many of these modules will be extracted to their own repositories. Another complexity is that the CDK is packaged using [jsii](https://github.com/aws/jsii) to multiple programming @@ -328,7 +328,7 @@ They can also be executed independently of the build script. From the root of a yarn lint ``` -The following linters are used - +The following linters are used: - [eslint](#eslint) - [pkglint](#pkglint) @@ -375,7 +375,7 @@ the [guidelines](./DESIGN_GUIDELINES.md). Here are a few useful commands: * `yarn awslint` in every module will run __awslint__ for that module. - * `yarn awslint list` prints all rules (details and rationale in the guidelines doc) + * `yarn awslint list` prints all rules (details and rationale in the guidelines doc). * `scripts/foreach.sh yarn awslint` will start linting the entire repo, progressively. Rerun `scripts/foreach.sh` after fixing to continue. * `lerna run awslint --no-bail --stream 2> awslint.txt` will run __awslint__ in all modules and collect all results into awslint.txt * `lerna run awslint -- -i ` will run awslint throughout the repo and @@ -391,7 +391,7 @@ examples are still accurate. Successfully building examples is also necessary to other supported languages (`C#`, `Java`, `Python`, ...). > Note that examples may use libraries that are not part of the `dependencies` or `devDependencies` of the documented -> package. For example `@aws-cdk/core` contains mainy examples that leverage libraries built *on top of it* (such as +> package. For example, `@aws-cdk/core` contains many examples that leverage libraries built *on top of it* (such as > `@aws-cdk/aws-sns`). Such libraries must be built (using `yarn build`) before **jsii-rosetta** can verify that > examples are correct. @@ -426,7 +426,7 @@ Each module also has an npm script called `cfn2ts`: ### scripts/foreach.sh This wonderful tool allows you to execute a command for all modules in this repo -in topological order, but has the incredible property of being stateful. this +in topological order, but has the incredible property of being stateful. This means that if a command fails, you can fix the issue and resume from where you left off. @@ -437,7 +437,7 @@ $ scripts/foreach.sh COMMAND ``` This will execute "COMMAND" for each module in the repo (cwd will be the directory of the module). -if a task fails, it will stop, and then to resume, simply run `foreach.sh` again (with or without the same command). +If a task fails, it will stop. To resume, simply run `foreach.sh` again (with or without the same command). To reset the session (either when all tasks finished or if you wish to run a different session), run: @@ -452,11 +452,11 @@ $ cd packages/my-module $ ../scripts/foreach.sh --up COMMAND ``` -This will execute `COMMAND` against `my-module` and all it's deps (in a topological order of course). +This will execute `COMMAND` against `my-module` and all its deps (in a topological order, of course). ### Jetbrains support (WebStorm/IntelliJ) -This project uses lerna and utilizes symlinks inside nested node_modules directories. You may encounter an issue during +This project uses lerna and utilizes symlinks inside nested `node_modules` directories. You may encounter an issue during indexing where the IDE attempts to index these directories and keeps following links until the process runs out of available memory and crashes. To fix this, you can run ```node ./scripts/jetbrains-remove-node-modules.js``` to exclude these directories. @@ -528,12 +528,12 @@ $ cd packages/@aws-cdk/aws-ec2 $ ../../../scripts/buildup ``` -Note that `buildup` uses `foreach.sh`, which means it's resumable. If your build fails and you wish to resume, just run +Note that `buildup` uses `foreach.sh`, which means it is resumable. If your build fails and you wish to resume, just run `buildup --resume`. If you wish to restart, run `buildup` again. ### Partial pack -Packing involves generating CDK code in the various target languages, and packaged up ready to be published to the +Packing involves generating CDK code in the various target languages and packaging them up to be published to their respective package managers. Once in a while, these will need to be generated either to test the experience of a new feature, or reproduce a packaging failure. @@ -569,14 +569,14 @@ Code... Now to test, you can either use `yarn test` or invoke nodeunit/jest directly: -Running nodeunit tests directly on a module +Running nodeunit tests directly on a module: ```console $ cd packages/@aws-cdk/aws-iam $ nodeunit test/test.*.js ``` -Running jest tests directly on a module +Running jest tests directly on a module: ```console $ cd packages/@aws-cdk/aws-iam $ jest test/*test.js @@ -599,7 +599,7 @@ One can use the `postinstall` script to symlink this repo: ``` This assumes this repo is a sibling of the target repo and will install the CDK as a linked dependency during -__yarn install__. +`yarn install`. ### Running integration tests in parallel @@ -713,7 +713,7 @@ can be used in these cases. #### Fixture Files Examples typed in fenced code blocks (looking like `'''ts`, but then with backticks -instead of regular quotes) will be automatically extrated, compiled and translated +instead of regular quotes) will be automatically extracted, compiled and translated to other languages when the bindings are generated. To successfully do that, they must be compilable. The easiest way to do that is using @@ -765,14 +765,14 @@ In order to offer a consistent documentation style throughout the AWS CDK codebase, example code should follow the following recommendations (there may be cases where some of those do not apply - good judgement is to be applied): -- Types from the documented module should be **un-qualified** +- Types from the documented module should be **un-qualified**: ```ts // An example in the @aws-cdk/core library, which defines Duration Duration.minutes(15); ``` -- Types from other modules should be **qualified** +- Types from other modules should be **qualified**: ```ts // An example in the @aws-cdk/core library, using something from @aws-cdk/aws-s3 @@ -880,7 +880,7 @@ this branch belongs to. To reduce merge conflicts in automatic merges between version branches, the current version number is stored under `version.vNN.json` (where `NN` is `majorVersion`) and changelogs are stored under `CHANGELOG.NN.md` (for -historical reasons, the changelog for 1.x is under `CHANGELOG.md`). When we +historical reasons, the changelog for 1.x is under `CHANGELOG.md`). When we fork to a new release branch (e.g. `v2-main`), we will update `release.json` in this branch to reflect the new version line, and this information will be used to determine how releases are cut. @@ -931,8 +931,7 @@ $ yarn build However, this will be time consuming. In this section we'll describe some common issues you may encounter and some more targeted commands you can run to resolve your issue. -* The compiler is throwing errors on files that I renamed/it's running old tests that I meant to remove/code coverage is - low and I didn't change anything. +#### The compiler is throwing errors on files that I renamed/it's running old tests that I meant to remove/code coverage is low and I didn't change anything. If you switch to a branch in which `.ts` files got renamed or deleted, the generated `.js` and `.d.ts` files from the previous compilation run are still around and may in some cases still be picked up by the compiler or test runners. @@ -943,7 +942,7 @@ Run the following to clear out stale build artifacts: $ scripts/clean-stale-files.sh ``` -* I added a dependency but it's not being picked up by the build +#### I added a dependency but it's not being picked up by the build You need to tell Lerna to update all dependencies: @@ -951,19 +950,18 @@ You need to tell Lerna to update all dependencies: $ node_modules/.bin/lerna bootstrap ``` -* I added a dependency but it's not being picked up by a `watch` background compilation run. +#### I added a dependency but it's not being picked up by a `watch` background compilation run. No it's not. After re-bootstrapping you need to restart the watch command. -* I added a dependency but it's not being picked up by Visual Studio Code (I still get red underlines). +#### I added a dependency but it's not being picked up by Visual Studio Code (I still get red underlines). The TypeScript compiler that's running has cached your dependency tree. After re-bootstrapping, restart the TypeScript compiler. Hit F1, type `> TypeScript: Restart TS Server`. -* I'm doing refactorings between packages and compile times are killing me/I need to switch between - differently-verionsed branches a lot and rebuilds because of version errors are taking too long. +#### I'm doing refactorings between packages and compile times are killing me/I need to switch between differently-verionsed branches a lot and rebuilds because of version errors are taking too long. Our build steps for each package do a couple of things, such as generating code and generating JSII assemblies. If you've done a full build at least once to generate all source files, you can do a quicker TypeScript-only rebuild of the @@ -996,7 +994,7 @@ $ CDK_TEST_BUILD=false lr test To debug your CDK application along with the CDK repository, 1. Clone the CDK repository locally and build the repository. See [Workflows](#workflows) section for the different build options. -2. Build the CDK application using the appropriate npm script (typically, `yarn build`) and then run the `link-all.sh` script as so - +2. Build the CDK application using the appropriate npm script (typically, `yarn build`) and then run the `link-all.sh` script as follows: ``` cd /path/to/cdk/app @@ -1036,7 +1034,7 @@ To debug your CDK application along with the CDK repository, } ``` - *Go [here](https://code.visualstudio.com/docs/editor/debugging#_launch-configurations) for more about launch configurations.* + *NOTE: Go [here](https://code.visualstudio.com/docs/editor/debugging#_launch-configurations) for more about launch configurations.* 6. The debug view, should now have a launch configuration called 'Debug hello-cdk' and launching that will start the debugger. 7. Any time you modify the CDK app or any of the CDK modules, they need to be re-built and depending on the change the `link-all.sh` script from step#2, may need to be re-run. Only then, would VS code recognize the change and potentially the breakpoint.