Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(docs): Add contributing and pr/issue templates #21

Merged
merged 5 commits into from
Apr 18, 2018
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions .github/ISSUE_TEMPLATE.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
<!--- Provide a general summary of the issue in the Title above -->
<!--- Format your PR title as such: <type>(<scope>): <subject> -->
<!--- according to [these guidelines](https://github.com/angular/angular.js/blob/master/DEVELOPERS.md#-git-commit-guidelines) -->
## Current Behavior
<!--- If describing a bug, tell us what is broken -->
<!--- If suggesting a change/improvement tell us what is missing from the SDK as it is currently -->

## Expected Behavior
<!--- If describing a bug, tell us how the SDK should behave without the bug -->
<!--- If you're suggesting changes, outline your idea here -->

## Possible Solution
<!--- Use this field to suggest bug solutions or change impementation details. Feel free to get technical. Optional, but helpful! -->

## Steps to Reproduce (for bugs)
<!--- Provide a link to a live example, or an unambiguous set of steps to -->
<!--- reproduce this bug. Include code to reproduce, if relevant -->

## Screenshots (if appropriate):

## Your Environment
<!--- Please include any relevant environmental information here. (SDK version, node version, browser etc.) -->
* SDK version:
* Node version:
* Browser name and version:
* Operating system:
22 changes: 22 additions & 0 deletions .github/PULL_REQUEST_TEMPLATE.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<!--- Provide a general summary of your changes in the Title above -->
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to mention that the PR needs to follow these commit guidelines: https://github.com/angular/angular.js/blob/master/DEVELOPERS.md#-git-commit-guidelines. Specifically the title needs to <type>(<scope>): <subject>


## Description
<!--- Describe your changes in detail -->
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Description needs to also include BREAKING CHANGE: if it's not backwards compatible (i.e. major bump)

<!--- Please include the phrase "BREAKING CHANGE:" here if your require a major release -->

<!--- Don't forget to note any issues here with "fixes #<issue number>" -->

## How Has This Been Tested?
<!--- Please describe in detail how you tested your changes. -->
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about adding:

For bug fixes, include regression unit tests that fail w/o the fix

For new features, include unit tests for the new functionality

<!--- For bug fixes, include regression unit tests that fail without the fix -->
<!--- For new features, include unit tests for the new functionality -->

## Screenshots (if appropriate):

## Checklist:
<!--- Please mark an `x` in all the boxes that apply. -->
<!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! -->
- [ ] I have read the **CONTRIBUTING** document.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if you can link to the CONTRIBUTING.md file? Mainly for other folks looking at the PR

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does it need to be a relative path since this file is nested in .github/ ? is there a way to test these readmes out before merging?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've honestly never done a template so I don't know what it would ultimately be relative too. This is all new to me so I'm excited! 🎉

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Upside, if it doesn't work we can quickly push a change that fixes it. 😅

- [ ] I have updated the documentation accordingly.
- [ ] I have added tests to cover my changes.
- [ ] I have run yarn validate to ensure that tests, typescript and linting are all in order.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one isn't really necessary since it gets run in Travis automatically for the PR

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think its still nice to have people run the tests though, might encourage more passing prs in our pr queue

71 changes: 71 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
#Contributing

Thank you for your interest in contributing to Eventbrite's Javascript SDK!

###Table of Contents
* [Workflow](#workflow)
* [Setup](#setup)
* [Branches](#using-branches-to-submit-changes)
* [Keeping up to date](#keeping-your-local-repo-up-to-date)
* [Creating issues](#creating-issues)
* [Working on and submitting changes](#working-on-and-submitting-changes)
* [Steps to submit](#steps-to-submit)

## Workflow

### Setup

1. This project uses [yarn](https://yarnpkg.com/en/) as a package management system. If you don't have it installed, you can follow the instructions [here](https://yarnpkg.com/lang/en/docs/install/)
1. Fork the repository. [need help?](https://help.github.com/articles/fork-a-repo/)
1. Clone your new forked repository to your local computer
1. Set the Eventbrite repository as your branches upstream branch
`git remote add upstream https://github.com/eventbrite/eventbrite-sdk-javascript.git`
1. Navigate to the root directory of your newly cloned repository
(for mac `cd /path/to/eventbrite-sdk-javascript`)
1. `yarn install` to install local dependencies
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TIL you can number them all 1 and they will autoincrement.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😅 i spent more time working out what @benmvp was trying to say here than i care to admit

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that is super odd, but really cool. I would not have guessed that would have worked.


### Using branches to submit changes
To work on changes to the Eventbrite repository, create a new branch on your local repository. `git checkout -b <your-new-branch-name>`


### Keeping your local repo up to date
To ensure your branch never gets out of sync with Eventbrite's master, ensure that you have your upstream set properly (see the [Setup](#setup) step)

1. `git checkout master` (you may have to [stash or commit][stash-docs] your local changes if on a new branch)
1. `git pull upstream master`
1. `git checkout <your-new-branch-name>`
1. `git rebase master`
1. If you've stashed changes, [unstash][stash-docs] them now, otherwise your branch should now be up to date

Always try to keep your master 'clean' by only pulling changes directly from upstream into your master branch and rebasing those changes onto your working branch.

It is always a good idea to pull the upstream branch in to your master branch before creating a new feature branch to work from. This will minimize the chances of encountering merge conflicts.

## Creating Issues
Create issues to file bugs, changes, and proposals.

Before opening a new issue, please [search][issues] to see if there has been previous discussion about the same feature or issue. If so, please contribute to the discussion there.

If nothing is found, feel free to [open a new issue][issues] and fill out the issue template to the best of your ability.

## Working on and submitting changes
When starting on improvements or new features that are non-trivial, it is always a good idea to first discuss the changes you wish to implement by [opening a github issue][issues] before getting started.

If you've found a bug or feature you'd like to work on in our [github issue tracker][issues], please comment on the issue to let others know that you'd like to work on it.

While implementing fixes, please try to change as little code as possible. This helps speed up the review process and helps diminish the chance of additional bugs.

Please try to conform to the coding style of the code base.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure how specific we want to be put, but either a link to the coding style or how they could verify that. ie yarn run lint && yarn run format

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code gets automatically formatted on commit, so they should be good there. And then linting runs in Travis as part of validate script. I actually don't think we're running prettier verification though. If someone bypasses prettier when committing they could actually put a PR up w/ unformatted code I believe...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think much like validating the title, we can probably find something to validate the the format is proper. Something like eslint-config-prettier should do the job.


###Steps to submit:

1. Please ensure that your changes are fully covered by one or more unit test(s).
1. Check to make sure that your changes are documented properly (inline comments for interesting lines, READMEs, etc.)
1. Run `yarn validate` to ensure that all tests pass, the linter is satisfied and your changes are typescript compliant.
1. PR titles must be prefixed by the type of changes the PR contains followed by the scope of what the pr touches. We are following the [angular commit guidelines](https://github.com/angular/angular.js/blob/master/DEVELOPERS.md#-git-commit-guidelines). Please use one of `feat, fix, docs, style, refactor, perf, test, chore` as the prefix. The scope is the the direct product your changes affect. Example: `chore(build): Add encrypted ssh key for semantic-release` because its a chore and it touches the build.
* For multiple scope items, you can comma separate 2 or 3 but if there are more than that please use a `*` instead.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

@kwelch kwelch Apr 9, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I believe that would be helpful.

We should probably add badge[s] to the readme that can give some of this context quickly as someone is looking at the package.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a semantic-release badge, but nothing (that I could find) for the angular commit convention. That'd be awesome

1. Please use a [closing issue keyword](https://help.github.com/articles/closing-issues-using-keywords/) to indicate the issue that your fix addresses in the description section of the pull request template. Example: `fixes #32` to close issue #32


[issues]: https://github.com/eventbrite/eventbrite-sdk-javascript/issues
[stash-docs]: https://git-scm.com/book/en/v1/Git-Tools-Stashing
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ Read more on [getting a token](https://www.eventbrite.com/developer/v3/api_overv

## Contributing

Coming soon...
Contributions are welcome! See [Contributing Guidelines](CONTRIBUTING.md) for more details.

## Project philosophy

Expand Down