Update contributor guidelines #280
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
broccolini
commented
Jul 25, 2017
|
|
||
| Here are a few things you can do that will increase the likelihood of your pull request being accepted: | ||
|
|
||
| - Follow the [style guide][style]. |
There was a problem hiding this comment.
removed this until we have public docs
shawnbot
reviewed
Jul 25, 2017
| 1. Fork and clone the primer/primer-css repository. | ||
| 2. Configure and install the dependencies: `npm install` | ||
| 3. Check out the dev branch `git checkout dev` | ||
| 3. Create a new branch from dev `git checkout -b my-branch-name` |
There was a problem hiding this comment.
It's not necessary, but we could collapse these two steps into one with git checkout -b my-branch-name dev.
shawnbot
reviewed
Jul 25, 2017
.github/CONTRIBUTING.md
Outdated
| 3. Check out the dev branch `git checkout dev` | ||
| 3. Create a new branch from dev `git checkout -b my-branch-name` | ||
| 4. Make your changes, add and commit them. | ||
| 5. Push your branch and open a pull request. Add a comment describing your proposed changes and request a review from `@primer/ds-core`. |
There was a problem hiding this comment.
This should ask them to open the PR against dev. We could point them at a URL like:
https://github.com/primer/primer-css/compare/dev...my-branch-name
shawnbot
reviewed
Jul 25, 2017
| 4. Make your changes, add and commit them. | ||
| 5. Push your branch and open a pull request. Add a comment describing your proposed changes and request a review from `@primer/ds-core`. | ||
| 6. Wait for CI tests to pass, if they do not, fix the errors or ask for help from `@primer/ds-core`. | ||
| 7. When CI tests pass, a new npm alpha release will be posted under the CI checks, you can use this npm version for testing in your project or with a GitHub site if you are staff. |
There was a problem hiding this comment.
We might format this step like so to be clearer that these are "sub-tasks" of the CI waiting game:
- Wait for CI tests to finish.
- If the tests pass, you should see a status check telling you which alpha version of
primer-cssyou can install with npm to test your work in other projects.- If the tests fail, review the logs and address any issues.
- If the builds fail for any other reason (as they occasionally do), they may need to be manually restarted.
shawnbot
suggested changes
Jul 26, 2017
.github/CONTRIBUTING.md
Outdated
| ## Feature requests | ||
|
|
||
| Feature requests are welcome. But take a moment to find out whether your idea fits with the scope and aims of the project. It's up to *you* to make a strong case to convince the project's developers of the merits of this feature. Please provide as much detail and context as possible. | ||
| Feature requests are welcome but take a moment to find out whether your idea fits with the scope and aims of the project. It's up to *you* to make a strong case to convince the project's developers of the merits of this feature. Please provide as much detail and context as possible. |
There was a problem hiding this comment.
I think a comma would make sense before "but" here.
.github/CONTRIBUTING.md
Outdated
| 6. Pat your self on the back and wait for your pull request to be reviewed and merged. | ||
| Anyone can open a pull request on Primer CSS. You do not need to work at GitHub or be a member of the org to open a pull request. | ||
|
|
||
| 1. Fork and clone the primer/primer-css repository. |
There was a problem hiding this comment.
Could we just say "this repository"?
There was a problem hiding this comment.
Sure, though I might link it. It seems obvious to us but with the new setup people might get muddled about where pr's should happen.
.github/CONTRIBUTING.md
Outdated
| 2. Configure and install the dependencies: `npm install` | ||
| 3. Check out the dev branch `git checkout dev` | ||
| 3. Create a new branch from dev `git checkout -b my-branch-name` | ||
| 4. Make your changes, add and commit them. |
There was a problem hiding this comment.
I'd vote for either "Make your changes, add, and commit them" (with an additional comma) or simply, "Make your changes and commit them".
shawnbot
reviewed
Jul 26, 2017
.github/CONTRIBUTING.md
Outdated
| 4. Make your changes and commit them. | ||
| 5. Push your branch and open a pull request against `dev`. Add a comment describing your proposed changes and request a review from `@primer/ds-core`. | ||
| 6. Wait for CI tests to finish. | ||
| - If the tests pass, you should see a status check telling you which alpha version of primer-css you can install with npm to test your work in other projects. |
There was a problem hiding this comment.
I think this sublist needs to be indented one more level to render correctly.
shawnbot
approved these changes
Jul 27, 2017
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I've updated the contribution guidelines for updating existing modules. I think this is enough for the public contribution guidelines for now. After seeing that there's a bunch of other stuff in here such as bug reporting, I think adding all the docs for githubbers will be a bit too much here so I think I'll add the detailed docs back in the style guide. When it's public, we can link to that from here too.
cc @primer/ds-core