Skip to content

Commit

Permalink
expand CONTRIBUTING to be more meaningful
Browse files Browse the repository at this point in the history
  • Loading branch information
@shiftkey
shiftkey committed Apr 21, 2017
1 parent f160091 commit 3a4fc93
Show file tree
Hide file tree
Showing 4 changed files with 207 additions and 33 deletions.
110 changes: 93 additions & 17 deletions CONTRIBUTING.md
View file
Original file line number Diff line number Diff line change
@@ -1,17 +1,93 @@
# How We Work

All changes are reviewed and the entire team is responsible for doing review.

The usual flow:

1. **Contributor** opens pull request.
2. If they're still working on it, they preface the title with `[WIP]` (**W**ork **I**n **P**rogress).
3. When it's ready for review, they comment on it saying so.
4. Suddenly a wild **reviewer** appears!
5. **Reviewer** assigns the PR to themselves.
6. **Reviewer** leaves line comments with suggestions or questions.
7. When the **reviewer** is done they comment on the PR with an emoji, meme, pokémon, or words.
8. The **contributor** responds to feedback, makes changes, etc.
9. When the **contributor** is ready for the **reviewer** to re-review, they comment on the PR with an emoji, meme, pokémon, or words.
10. Goto 6 until both parties are happy with the PR.
11. The **reviewer** hits the big green merge button and deletes the branch.
# Contributing to GitHub Desktop

Please take some time out to read these resources before you start contributing
changes to GitHub Desktop.

## Setup

You will need to install these tools:

- [Nodejs](https://nodejs.org) v7 is preferred as it's the version embedded into Electron
- **Windows developers**: Make sure you allow the Node.js installer to add
node to the PATH, it'll make life much easier for you.
- Python 2.7 - [Windows](https://www.python.org/downloads/windows/), [macOS](https://www.python.org/downloads/mac-osx/)
- **Windows developers**: Let python install into the default suggested path
(`c:\Python27`), otherwise you'll have to configure node-gyp manually with
the path which is annoying.
- **macOS:** Xcode and Xcode Command Line Tools (Xcode -> Preferences -> Downloads)
- **Windows:** Visual Studio 2015 or [Visual C++ Build Tools](http://go.microsoft.com/fwlink/?LinkId=691126)
- Run `npm config set msvs_version 2015` after installing the build tools

With these things installed, open a shell and validate your output to these commands looks like this:

```
> node -v
v7.8.0
> npm -v
4.2.0
> python --version
Python 2.7.13
```

We also have some [additional resources](./docs/contributing/tooling.md) to help you configure
favourite editor to work nicely with the GitHub Desktop repository.

## Building

After cloning the repository, the typical workflow to get the app up and running
is as follows:

* Run `npm install` to get all required dependencies on your machine.
* Run `npm run build:dev` to make a development build of the app.
* Run `npm start` to launch the application. Changes will be compiled in the
background. The app can then be reloaded to see the changes (Ctrl/Command+R).

If you've made changes in the main-process folder you need to run `npm run
rebuild:dev` and then `npm run start` for these changes to be reflected.

If you're still encountering issues with building, refer to our
[troubleshooting](./docs/contributing/troubleshooting.md) guide for more common
problems.

## Running tests

- `npm test` - Runs all unit and integration tests
- `npm run test:unit` - Runs all unit tests
- `npm run test:integration` - Runs all integration tests

**Pro Tip:** If you're only interested in the results of a single test and don't
wish to run the entire test suite to see it you can pass along a search string
in order to only run the tests that match that string.

```
npm run test:unit -- --grep CloneProgressParser
```

This example will run all tests matching `CloneProgressParser`.

## Debugging

Electron ships with Chrome Dev Tools to assist with debugging, profiling and
other measurement tools.

1. Run the command `npm start` to launch the app
2. Open _Chrome Dev Tools_

[React Dev
Tools](https://chrome.google.com/webstore/detail/react-developer-tools/fmkadmapgofadopljbjfkapdkoienihi?hl=en)
should automatically install itself on first start when in development mode.

If you would also like to use [Devtron](http://electron.atom.io/devtron/), run
the command `require('devtron').install()` inside of the console in _Chrome Dev
Tools_.

## The Next Steps

You've made it to here, so let's give you some other things to read to get you started:

- [Up for Grabs](./docs/process/up-for-grabs.md) - we've marked some tasks in
the backlog that are ideal for external contributors
- [Code Reviews](./docs/process/reviews.md) - some notes on how the team does
code reviews
48 changes: 48 additions & 0 deletions docs/contributing/tooling.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
# Tooling Support for GitHub Desktop

## [Atom](https://atom.io/)

Here are some plugins you should also install:

* [atom-typescript](https://atom.io/packages/atom-typescript) - Syntax highlighting and intellisense for TypeScript
* [atom-build-npm-apm](https://atom.io/packages/build-npm-apm) - Lets you invoke all npm scripts straight from the editor by pressing F7 (requires [atom-build](https://atom.io/packages/build))
* [linter](https://atom.io/packages/linter) and [linter-tslint](https://atom.io/packages/linter-tslint) - Shows linter errors and warning in the editor

## [Visual Studio Code](https://code.visualstudio.com/)

The Desktop repository includes a list of extensions that we recommend:

1. While in the _Extension_ view, select *Show Workspace Recommended Extensions* from the dropdown menu
2. Install all the extensions

We also support debugging the running app inside VSCode:

1. Run the command `npm run debug`
2. Select the _Debug_ view from the view bar
3. Select the process you would like to attach to (this will usually be the _Renderer_ process)
4. Press `F5` or the green play button

![2017-02-07_15-24-23](https://cloud.githubusercontent.com/assets/1715082/22712204/90ca44fa-ed49-11e6-9110-ffa9c1d4f752.jpg)




## Debugging

### Chrome

1. Run the command `npm run start`
2. Open _Chrome Dev Tools_

[React Dev Tools](https://chrome.google.com/webstore/detail/react-developer-tools/fmkadmapgofadopljbjfkapdkoienihi?hl=en) should automatically install itself on first start. If you would also like to use [Devtron](http://electron.atom.io/devtron/), run the command `require('devtron').install()` inside of the console in _Chrome Dev Tools_.

### VS Code

1. Run the command `npm run debug`
2. Select the _Debug_ view from the view bar
3. Select the process you would like to attach to (this will usually be the _Renderer_ process)
4. Press `F5` or the green play button

![2017-02-07_15-24-23](https://cloud.githubusercontent.com/assets/1715082/22712204/90ca44fa-ed49-11e6-9110-ffa9c1d4f752.jpg)


36 changes: 36 additions & 0 deletions docs/contributing/troubleshooting.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
# Known Issues

Here's a non-exhaustive list of environmental issues that you may encounter
while working on GitHub Desktop:

### Issues compiling node-keytar on Windows

If keytar fails to build on Windows with the following error during `npm install`:

```
npm ERR! keytar@3.0.2 install: `node-gyp rebuild`
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the keytar@3.0.2 install script 'node-gyp rebuild'.
npm ERR! Make sure you have the latest version of node.js and npm installed.
npm ERR! If you do, this is most likely a problem with the keytar package,
npm ERR! not with npm itself.
npm ERR! Tell the author that this fails on your system:
npm ERR! node-gyp rebuild
npm ERR! You can get information on how to open an issue for this project with:
npm ERR! npm bugs keytar
npm ERR! Or if that isn't available, you can get their info via:
npm ERR! npm owner ls keytar
```

Make sure you're using npm >= 2.15.9

```
PS> npm -g install npm@latest
```

and run `npm install` again

See https://github.com/atom/node-keytar/issues/45 and
https://github.com/nodejs/node-gyp/issues/972 for more information.

46 changes: 30 additions & 16 deletions docs/process/reviews.md
View file
Original file line number Diff line number Diff line change
@@ -1,14 +1,32 @@
# Code Reviews
# The Review Process

Some notes on how the Desktop core team review incoming pull requests
This is the typical flow:

## The Flow
1. **Contributor** opens pull request.
2. If they're still working on it, they preface the title with `[WIP]` (**W**ork **I**n **P**rogress).
3. When it's ready for review, they comment on it saying so.
4. Suddenly a wild **reviewer** appears!
5. **Reviewer** assigns the PR to themselves.
6. **Reviewer** leaves line comments with suggestions or questions.
7. When the **reviewer** is done they comment on the PR with an emoji, meme,
pokémon, or words to that effect.
8. The **contributor** responds to feedback, makes changes, etc.
9. When the **contributor** is ready for the **reviewer** to re-review, they
comment on the PR with an emoji, meme, pokémon, or words to that effect.
10. Goto 6 until both parties are happy with the PR.
11. The **reviewer** hits the big green merge button and deletes the branch (if
applicable).

The rest of this document goes into more details around this flow.

## Notes for Contributors

### Work-in-Progress

You should open pull requests early, especially for large changes. This means
continuous integration tests are run earlier on, and any regressions can be
caught before humans get involved.
We recommend open pull requests early - ideally as soon as you have somethign to
show off. This is especially helpful for large changes, as continuous
integration tests are run earlier on, and any regressions can be caught before
humans get involved.

Until the code is ready for review, you can prefix the PR title with [WIP] to
indicate that it's under development.
Expand All @@ -24,19 +42,15 @@ understand quickly what's changed.
We're not that fussy about the history, but to make reviewing easier here's
some general tips:

- make small, meaningful commits -
- write good commit messages - these help the reviewer to understand the changes
- make small, meaningful and logical commits - these make the review process easier
- [write good commit messages](https://chris.beams.io/posts/git-commit/) -
these help the reviewer to understand the changes
- keep up to date with `master` - not only does this address potential merge
conflicts, it ensures you're integrating with the latest code

Please also keep your branch up to date with `master` to ensure everything
integrates nicely - we prefer you merge `master` into your branch, but for small
When merging, we prefer you merge `master` into your branch, but for small
PRs a rebase is fine.

### Tell Us When It's Ready

Feeling like it's ready to be reviewed? Add a comment to the Pull Request to
the effect of "Ready for review! :dog:" - this let's us know it's time to start
with a thorough review.

### At Least One Assignee

While many people can be involved with a review, there's generally one person
Expand Down

0 comments on commit 3a4fc93

Please sign in to comment.