|
1 | 1 | # Contributing |
2 | 2 |
|
3 | | -At Reaction Commerce, we're dedicated to the open source community. In fact, we've designed our entire platform and business to grow from the passion and creativity that an open source community ignites. We've already attracted a small, dedicated team of open source contributors, and there's always room for more. |
| 3 | +At Mailchimp Open Commerce (from now MOC for brevity) we're dedicated to the open source community. In fact, we've designed our entire platform and business to grow from the passion and creativity that an open source community ignites. |
4 | 4 |
|
5 | | -If you'd like to join us, check out our detailed [Contributing Guide](https://docs.reactioncommerce.com/reaction-docs/trunk/contributing-to-reaction). |
| 5 | +## At a glance |
6 | 6 |
|
| 7 | +We've already attracted a small, dedicated team of open source contributors, and there's always room for more. If you'd like to join us, here's how to get started. |
| 8 | + |
| 9 | +### Step 1: Get MOC running |
| 10 | + |
| 11 | +If you haven't already, get MOC running locally. |
| 12 | + |
| 13 | +Instructions are [here](/developer/open-commerce/guides/quick-start) for Windows, Mac OSX and Linux. |
| 14 | + |
| 15 | +### Step 2: Find or open an issue |
| 16 | + |
| 17 | +There are two ways to go about contributing to MOC: file a bug or work on an issue that is already created and vetted by the team. |
| 18 | + |
| 19 | +#### File a bug |
| 20 | +Before you file a bug, please search existing issues first. |
| 21 | +Are you looking for support instead? Please go to our [Discord chat](https://discord.gg/Bwm63tBcQY) instead. |
| 22 | +Make sure to follow the issue template. |
| 23 | +Once your bug issue is filed, the community team will evaluate and prioritize using the following label/criteria: |
| 24 | + |
| 25 | +- **impact-critical** (do now): Blocks core functionality which would include checking out, processing orders, adding a product, etc. |
| 26 | +- **impact-major** (do next): Blocks important functionality but there is a workaround or the problem doesn't inhibit shopping/purchasing. |
| 27 | +- **impact-minor** (do eventually): Impacts peripheral functionality or there is a reasonable workaround (UI glitches, etc). |
| 28 | +Once it's been triaged and verified, a Community Engineering team member will work on it according the above criteria. |
| 29 | + |
| 30 | +#### Find an issue and claim it |
| 31 | +Explore the *Help Wanted* or *Good First Issue* issues on our GitHub repos. |
| 32 | +If you find something you want to work on, let us know right there in the comment with how you want to approach the problem. |
| 33 | +If you are a first-time contributor, also mention the @reactioncommerce/community team in the comment so you can request to be made a contributor. |
| 34 | + |
| 35 | + |
| 36 | +### Step 3: Prepare a pull request for review |
| 37 | +Branch off `trunk` for your PR branch, unless you're fixing an urgent issue to a specific release that is still supported. |
| 38 | + |
| 39 | +Once your branch fulfills the issue it tackles, you are ready to create a pull request (PR). |
| 40 | + |
| 41 | +Select `trunk` as the base branch (the branch you want your changes pulled into). |
| 42 | + |
| 43 | +Fill out the pull request template |
| 44 | +Before you are ready for a team code review, you will also have to fill out the following sections in the template: |
| 45 | + |
| 46 | +- **Resolves** - Note issue number: Link to the GitHub issue number. |
| 47 | +If you're resolving an unreported bug, note: Resolves unreported issue. |
| 48 | +We do not accept PRs for features without issues. |
| 49 | +- **Impact** - Choose from one of the following: |
| 50 | + - **breaking**: introduces breaking changes to the app. |
| 51 | + - **critical**: resolves a critical bug blocking core functionality. Examples include browsing products, adding products to cart, checking out, processing orders, etc. |
| 52 | + - **major**: resolves a major bug or introduces significant new feature. |
| 53 | + - **minor**: resolves a minor bug, minor changes to the app, or minor new feature |
| 54 | + |
| 55 | +- **Type** - Choose from one of the following: |
| 56 | + - **feature**: A new feature or functionality |
| 57 | + - **bugfix**: A bug fix |
| 58 | + - **performance**: A code change that improves performance |
| 59 | + - **style**: Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc) |
| 60 | + - **test**: Adding missing or correcting existing tests |
| 61 | + - **refactor**: A code change that neither fixes a bug nor adds a feature |
| 62 | + - **docs**: Documentation only changes |
| 63 | + - **chore**: Changes to the build process or auxiliary tools and libraries such as documentation generation |
| 64 | + |
| 65 | +- **Issue Description** - Describe the issue this PR is solving with the knowledge you've gained by fixing it. This may differ from the original ticket as you now have more information at your disposal. |
| 66 | +Include additional information gathered during the process of resolving the ticket that might be helpful to reviewers or other users who might encounter the same problem. |
| 67 | +Include all information necessary to understand the issue this PR resolves so that the reviewer does not need to look at the original ticket. |
| 68 | + - **Solution** - Summarize your solution to the problem. Please include short descriptions of any solutions you tested before arriving at your final solution. This will help reviewers know why you decided to solve this problem in this particular way and will speed up the review process. |
| 69 | +Note new dependencies: If you have introduced any new dependencies, please list them, explain how they are used in your solution and any other libs that you considered. |
| 70 | +- **Breaking changes** - List breaking changes, or otherwise list none. |
| 71 | + - Changing file names |
| 72 | + - Moving files |
| 73 | + - Deleting files |
| 74 | + - Renaming functions or exports |
| 75 | + - Changes to code which might cause previous versions of MOC or third-party code not to work as expecteNote any work that you did to mitigate the effect of any breaking changes such as creating migrations, deprecation warnings, etc. |
| 76 | + |
| 77 | +- **Testing Instructions** - Write instructions for testing your changes. You can assume that reviewers know how to start the app and how to perform basic setup tasks. For any task where there may be multiple ways to do something, be explicit. (e.g. there are several ways to "Create a Product" and many options once created before a product is published). |
| 78 | +The steps you list should guide the reviewer through testing the feature or fix you've implemented. These steps will generally be very similar to the reproduction steps in the issue. |
| 79 | + |
| 80 | +#### Pass all tests |
| 81 | +As soon as your PR is pushed, automated tests run to ensure: |
| 82 | + |
| 83 | +`npm run lint`: Code style is correct |
| 84 | +`npm run test`: All unit and integration tests pass |
| 85 | + |
| 86 | +### Step 4: PR review process begins |
| 87 | +The Community team triages all new pull requests as soon as the PR is complete. |
| 88 | + |
| 89 | +#### PR gets reviewed |
| 90 | +The team reviews code quality rules including: |
| 91 | + |
| 92 | +- **PR template**: If the PR doesn't follow the our template, reject and point the author of the PR to this doc. |
| 93 | + |
| 94 | +- **Issue description**: Use this information as the starting point for your review. If something is not clear, reject the PR and ask for clarity by requesting changes. While the original issue may have useful information, the PR should contain the most up to date representation of the issue. |
| 95 | + |
| 96 | +- **Solution**: Use this information to help determine a path to test this PR. Research any included packages or techniques that may have been used that you're not familiar with. Ask questions if you're confused. |
| 97 | + |
| 98 | +- **Breaking changes**: Test by applying this patch to an existing install of MOC with existing users, orders, carts, etc. Specifically, test any parts of the app where the breaking change is involved and any data set that is involved in a migration. |
| 99 | + |
| 100 | +- **Testing**: Run through the author's steps to verify that it works as they've tested it. Then run through the app on your own as you would test it. Run through the app as many times as you feel comfortable before approving or requesting changes. |
| 101 | + |
| 102 | +- **Readability**: the linter will help with this, but call out anything that is difficult to understand or that you feel needs comments |
| 103 | + |
| 104 | +- **Documentation**: all code added or touched should have proper JSDoc, any new functionality should be documented, as outlined in JSDoc Style Guide. |
| 105 | + |
| 106 | +- **Security**: Code should only be usable by users with the correct roles. Any data published should be filtered to ensure that only users with the correct roles for the correct shops have access to it. |
| 107 | + |
| 108 | +- **Performance**: Code should be written with performance in mind. Data publications should only publish data necessary to accomplish the specific goal at hand. |
| 109 | + |
| 110 | +- **Tests**: Any new functionality should include tests |
| 111 | + |
| 112 | +- **Dependencies**: Any newly introduced dependencies should be updated to the latest version. No Meteor dependencies. |
| 113 | + |
| 114 | +- **i18n**: All static copy should use i18next. Include definitions in the appropriate en.json file. |
| 115 | + |
| 116 | +- **a11y**: Code should be accesibility compliant. |
| 117 | + |
| 118 | +Reviewers will note any changes that they will want to QA in the app, even if they aren't listed in the testing steps (e.g if the code changes a cart button, ensure that the button still works). |
| 119 | + |
| 120 | +#### PR is ready to merge |
| 121 | +Congrats - Once you have all the green lights with an approved PR, you are ready to merge. |
| 122 | + |
| 123 | +Does your new feature require new user documentation or developer documentation? Make an issue for that in *reaction-docs*. |
| 124 | + |
| 125 | +### Step 5: Congrats! It's merged. What happens next? |
| 126 | +Now that your PR is merged, the feature will be released in the next release. |
0 commit comments