Skip to content

Latest commit

 

History

History
307 lines (230 loc) · 12.6 KB

CONTRIBUTING.md

File metadata and controls

307 lines (230 loc) · 12.6 KB

Welcome!

I'm so glad you've found this project interesting and useful enough that you'd like to contribute to its development.

Please take time to review the policies and procedures in this document prior to making and submitting any changes.

This guide was drafted with tips from Wrangling Web Contributions: How to Build a CONTRIBUTING.md and with some inspiration from the Atom project's CONTRIBUTING.md file.

Table of contents

Quick links

Contributor License Agreement

Per the GitHub Terms of Service, be aware that by making a contribution to this project, you agree:

  • to license your contribution under the same terms as this project's license, and
  • that you have the right to license your contribution under those terms.

See also: "Does my project need an additional contributor agreement? Probably not."

Code of conduct

Harrassment or rudeness of any kind will not be tolerated, period. For specifics, see the CODE_OF_CONDUCT file.

Reporting issues

Before reporting an issue, please use the search feature on the issues page to see if an issue matching the one you've observed has already been filed.

If you do find one...

Do not add a +1 comment!

If you find an issue that interests you, but you have nothing material to contribute to the thread, use the Subscribe button on the right side of the page to receive notifications of further conversations or a resolution. Comments consisting only of "+1" or the like tend to clutter the thread and make it more painful to follow the discussion.

If you do have something to add to the conversation, or don't find a matching issue...

Update an existing issue or file a new one

Try to be as specific as possible about your environment and the problem you're observing. At a minimum, include:

  • The version of the slack-github-issues package you're using
  • Command line steps, code snippets, or automated tests that reproduce the issue

Updating documentation

If you've a passion for writing clear, accessible documentation, please don't be shy about sending pull requests! The documentation is just as important as the code.

Also: no typo is too small to fix! Really. Of course, batches of fixes are preferred, but even one nit is one nit too many.

Development environment setup

Install Node.js per step 1 of the "Installation and usage" instructions from the README. You don't need to create an app instance, a Slack bot user, a GitHub user, a configuration file, or environment variables until you intend to deploy your app.

You will also need Git installed on your system. If you are not familiar with Git, you may wish to reference the Git documentation.

Once Node.js and Git are installed, clone this repository and ensure your development environment is in a good state:

$ git clone git@github.com:mbland/slack-github-issues.git
$ cd slack-github-issues
$ ./go

For information on the ./go script and its commands, run ./go help.

After making changes, run ./go lint and ./go test frequently. Add new tests in the tests directory for any new functionality, or to reproduce any bugs you intend to fix.

If your shell is Bash, you may wish to make an alias for the ./go script for convenience. For example:

$ eval "$(./go env sgi)"

will create a shell function called sgi that will make the ./go commands from the project available from any directory, and that will provide tab-completion for the script and any of its commands that implement completion. Run ./go help env and ./go env for details.

Workflow

The basic workflow for submitting changes resembles that of the GitHub Git Flow, except that you will be working with your own fork of the repository and issuing pull requests to the original.

  1. Fork the repo on GitHub (look for the "Fork" button)
  2. Clone your forked repo to your local machine
  3. Create your feature branch (git checkout -b my-new-feature)
  4. Develop and test your changes as necessary.
  5. Commit your changes (git commit -am 'Add some feature')
  6. Push to the branch (git push origin my-new-feature)
  7. Create a new GitHub pull request for your feature branch based against the original repository's master branch
  8. If your request is accepted, you can delete your feature branch and pull the updated master branch from the original repository into your fork. You may even delete your fork if you don't anticipate making further changes.

Testing

  • Continuous integration status: Continuous integration status

  • Static analysis status: Static analysis status

  • Coverage status: Coverage status

  • Continuous integration status: Continuous integration status

  • Coverage status: Coverage Status

No bug fixes or new features will be accepted without accompanying tests. Period.

Any changes that break the continuous integration build must be fixed or rolled back immediately.

This project uses the Mocha test framework, the Chai assertion library, and the Chai-as-promised assertions for Promises library to write and run tests. All tests are in the tests/ directory and are run using the ./go test command.

The ./go test command has a very flexible syntax for running a subset of test suites and passing command line options to Mocha. Enabling tab completion via ./go env is highly encouraged. See ./go help test for more information.

Before sending your code for review, make sure to run the entire test suite via ./go test --coverage to make sure everything passes and your changes are adequately covered by new and existing tests.

Coding conventions

Formatting

  • Most of the rules are automated via the .eslintrc file. See the ESLint web site for the User guide and Rules definitions.
  • Notice the maximum line length is 80 characters. (Yes, the maintainer is a dinosaur who likes viewing files side-by-side in a 161-column terminal window.)

The following are intended to prevent too-compact code:

  • Declare only one variable per var line
  • Do not use one-line if, for, or while statements.
  • Do not write functions entirely on one line.
  • For case statements: put each pattern on a line by itself; put each command on a line by itself; put the break; terminator on a line by itself.

Confession: I have used one-liners like crazy in the past. Looking back at my own code, I've found them difficult to understand. Spreading out declarations, statements, and functions makes the code easier to follow, as the behavior is more explicit. It also makes it more grep-pable, as "one thing per line" makes it easier to find, count, and possibly transform things.

Naming

  • Constants and globals should be in ALL_CAPS. Most should be defined in lib/constants.js.

ES6+ features

  • Use only Promises, which have been supported since Node.js v0.12. Since this is library code, we want to support the earliest Node version possible. You can see the latest Node compatibility info at node.green.

Function declarations

  • Don't use arrow functions. (See [ES6+ features][#es6+-features].)
  • Don't use Function.bind(), as its performance tends to be sub-par. Define explicit closures around a var impl = this; declaration (where impl may be different depending on the context) instead.
  • Try to keep function expression implementations very short, optimally one line. In other words, most function expressions should delegate to another function immediately.
    • This is for two reasons. One, it's typically easier to follow the logic when a dynamically-generated function delegates to a more permanent one. Second, it may enable Node.js/V8 to better optimize the permanent function. (Of course, I haven't measured this yet...)
  • Functions containing a try/catch block should also remain very small, and delegate to other functions as appropriate.
    • This is again to aid readability, and also performance, as it is known that Node.js/V8 will not optimize a function containing a try/catch block.

Conditionals and loops

  • Always put the opening of the statement (including the opening brace) on the first line, start the body on the second line, and close the statement with a single curly brace (or else clause) on the last line.
  • For else clauses, close the previous statement and open the else statement on the same line.
  • Use the ternary operator (condition ? true_value : false_value) only if it's extremely clear what the expression is doing. The maintainer will ultimately decide whether its use is merited.

Output

  • All output should go through the logger member of the ReactionIssueFiler variable. It should be instantiated using the Logger class, which is a wrapper that adheres to the log npm interface.

Gotchas

  • With Promises, ensure that one of resolve or reject is always called, otherwise the Promise will not ever produce a result.
    • Alternatively, it may be desirable or necessary to use return Promise.resolve() or return Promise.reject() as an alternative.
  • When creating Promise chains, always remember to return the next Promise from the previous Promise's .then() or .catch() handler. Otherwise the handler will return undefined, which will break the chain. (The next Promise will still execute, but whatever follows it will not wait for it to resolve.)

Open Source License

This software is made available as Open Source software under the ISC License. For the text of the license, see the LICENSE file.