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

ryanwholey · 2018-04-05T19:59:57Z

First pass at writing contributing docs and the issue/pr templates! Looking for feedback on clarity rewrites, topics to add and template information.

This pr adds a contributing doc outline what is expected of a developer looking to contribute.
Also included are two template MD files that outline information we'd like to see in our submitted issues and prs.

Let me know what you think!

fixes #16

kwelch · 2018-04-05T20:05:56Z

CONTRIBUTING.md

+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.


nit: File should probably be lowercase.

kwelch · 2018-04-05T20:09:14Z

CONTRIBUTING.md

+
+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.


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

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...

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.

kwelch · 2018-04-05T20:16:10Z

Some minor nits, but all in all looks good.

benmvp

This is amazing!!!! So excited for this. This will be great for people w/in EB let alone the public. Left a few comments here and there :)

benmvp · 2018-04-05T20:18:38Z

.github/ISSUE_TEMPLATE.md

+<!--- If you're suggesting changes, outline your idea here -->
+
+## Possible Solution
+<!--- Optional, but helpful! Use this field to suggest bug solutions or change impementation details. Feel free to get technical. -->


Can you move the

Optional, but helpful!

part to the end? I really want folks to do this part 😄

benmvp · 2018-04-05T20:29:47Z

.github/PULL_REQUEST_TEMPLATE.md

@@ -0,0 +1,18 @@
+<!--- Provide a general summary of your changes in the Title above -->


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>

benmvp · 2018-04-05T20:30:17Z

.github/PULL_REQUEST_TEMPLATE.md

+
+## Description
+<!--- Describe your changes in detail -->
+<!--- Don't forget to note any issues here with fixes #<issue number> -->


Can you put quotes around "fixes #<issue number>"

benmvp · 2018-04-05T20:32:27Z

.github/PULL_REQUEST_TEMPLATE.md

+<!--- 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. -->


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

benmvp · 2018-04-05T20:33:15Z

.github/PULL_REQUEST_TEMPLATE.md

+<!--- Provide a general summary of your changes in the Title above -->
+
+## Description
+<!--- Describe your changes in detail -->


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

benmvp · 2018-04-05T20:37:50Z

CONTRIBUTING.md

+### 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 your local changes if on a new branch)


link to this doc on stashing? https://git-scm.com/book/en/v1/Git-Tools-Stashing

benmvp · 2018-04-05T20:40:33Z

CONTRIBUTING.md

+
+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.


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...

benmvp · 2018-04-05T20:41:50Z

CONTRIBUTING.md

+
+###Steps to submit:
+
+1. Please ensure that your changes are fully covered by one or more unit test/s.


lol thank you, that looked weird to me

benmvp · 2018-04-05T20:42:49Z

CONTRIBUTING.md

+2. Check to make sure that your changes are documented properly (inline comments for interesting lines, READMEs, etc.)
+3. Run `yarn validate` to ensure that all tests pass, the linter is satisfied and your changes are typescript compliant.
+4. PR titles must be prefixed by the type of changes the PR contains followed by the scope of what the pr touches. 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.


Should we link to https://github.com/angular/angular.js/blob/master/DEVELOPERS.md#-git-commit-guidelines ?

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.

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

benmvp · 2018-04-05T20:43:51Z

README.md

@@ -24,7 +24,7 @@ Read more on [getting a token](https://www.eventbrite.com/developer/v3/api_overv

 ## Contributing

-Coming soon...
+Check out our contributing document [here](https://github.com/eventbrite/eventbrite-sdk-javascript/blob/master/CONTRIBUTING.md)


Can you do:

Contributions are welcome! See Contributing Guidelines for more details.

Also you can just do CONTRIBUTING.md as the URL. You don't have to do the full URL

kwelch · 2018-04-10T14:09:54Z

CONTRIBUTING.md

 (for mac `cd /path/to/eventbrite-sdk-javascript`)
-5. `yarn install` to install local dependencies
+1. `yarn install` to install local dependencies


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

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

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

benmvp

Looks good to me! Thanks 🎉

benmvp · 2018-04-10T19:30:59Z

.github/PULL_REQUEST_TEMPLATE.md

+## 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.


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

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?

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! 🎉

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

ryanwholey added 3 commits April 5, 2018 09:12

contributing

f9366fb

first pass

3647706

update issue template

e52e033

kwelch approved these changes Apr 5, 2018

View reviewed changes

benmvp reviewed Apr 5, 2018

View reviewed changes

update per kyle and bens comments

5e988e9

kwelch reviewed Apr 10, 2018

View reviewed changes

benmvp approved these changes Apr 10, 2018

View reviewed changes

link contributing from pr template?

30d9738

rwholey-eb merged commit d309ea8 into eventbrite:master Apr 18, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

ryanwholey commented Apr 5, 2018

kwelch Apr 5, 2018

kwelch Apr 5, 2018

benmvp Apr 5, 2018

kwelch Apr 9, 2018

kwelch commented Apr 5, 2018

benmvp left a comment

benmvp Apr 5, 2018

benmvp Apr 5, 2018

benmvp Apr 5, 2018

benmvp Apr 5, 2018

benmvp Apr 5, 2018

benmvp Apr 5, 2018

benmvp Apr 5, 2018

benmvp Apr 5, 2018

ryanwholey Apr 10, 2018 •

edited

Loading

benmvp Apr 5, 2018

kwelch Apr 9, 2018 •

edited

Loading

benmvp Apr 9, 2018

benmvp Apr 5, 2018

kwelch Apr 10, 2018

ryanwholey Apr 10, 2018

kwelch-eb Apr 10, 2018

benmvp left a comment

benmvp Apr 10, 2018

ryanwholey Apr 11, 2018

benmvp Apr 11, 2018

kwelch Apr 11, 2018


		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.

		@@ -0,0 +1,18 @@
		<!--- Provide a general summary of your changes in the Title above -->


		###Steps to submit:

		1. Please ensure that your changes are fully covered by one or more unit test/s.

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

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

Conversation

ryanwholey commented Apr 5, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kwelch commented Apr 5, 2018

benmvp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ryanwholey Apr 10, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kwelch Apr 9, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benmvp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ryanwholey Apr 10, 2018 •

edited

Loading

kwelch Apr 9, 2018 •

edited

Loading