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

Create contributing guidelines #16

Closed
ryanwholey opened this issue Mar 29, 2018 · 12 comments · Fixed by #21
Closed

Create contributing guidelines #16

ryanwholey opened this issue Mar 29, 2018 · 12 comments · Fixed by #21
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@ryanwholey
Copy link
Contributor

We should include some contributing guidelines to replace the "coming soon" portion of our docs!

Probably a CONTRIBUTING.md file would be good and then link it from the README.md.

Things to include:

  • semantic commit messages i think this is it?
  • open an issue to discuss non trivial changes first
  • commit message outline?
  • update docs and tests
@benmvp
Copy link
Contributor

benmvp commented Mar 29, 2018

Yeah we need lots of docs! 😄 CONTRIBUTING.md is a good start.

Here's the resource for the commit message: https://github.com/angular/angular.js/blob/master/DEVELOPERS.md#-git-commit-guidelines

@BenAtEventbrite BenAtEventbrite added the enhancement New feature or request label Mar 29, 2018
@kwelch
Copy link
Contributor

kwelch commented Apr 2, 2018

Should we use something like commitizen to help work flow/force consistency in commit messages.

@benmvp
Copy link
Contributor

benmvp commented Apr 2, 2018

@kwelch so I'd love to use it for PRs. But my understanding is that it works on every commit, which would be annoying because we probably make a bunch of local commits before putting up the PR. And since we only do rebase merges of PRs, the only title that really matters is the title of the PR...

@ryanwholey
Copy link
Contributor Author

Britecharts has set up templating for issues and pull requests which has worked really well for us! I would be in favor of adding something like that here.

Here's a link to one of the prs setting it up in britecharts

@benmvp
Copy link
Contributor

benmvp commented Apr 2, 2018

Sounds like you should take this issue then 😉

@ryanwholey
Copy link
Contributor Author

so down 👍

@rwholey-eb rwholey-eb assigned rwholey-eb and ryanwholey and unassigned rwholey-eb Apr 2, 2018
@kwelch
Copy link
Contributor

kwelch commented Apr 3, 2018

@benmvp Good point, that would be very annoying locally. The Templates are a great approach. On react-slingshot, we have a PR template that includes a checklist, it is for both the submitter and the reviewer.

@benmvp
Copy link
Contributor

benmvp commented Apr 3, 2018

That one is nice too @kwelch! I feel like we can merge them together to ensure enough information is included in the PRs. But the real key is making sure the title format is followed (as well as mentioning a breaking change)

@kwelch
Copy link
Contributor

kwelch commented Apr 4, 2018

Is there a GitHub bot that will check the PR title similar to what we have in phab? Or can we build that into the travis script?

This looks like it could be helpful in verifying and validating both messages and PR titles. https://github.com/uber-workflow/probot-app-pr-title

@benmvp
Copy link
Contributor

benmvp commented Apr 5, 2018

I was thinking of going the approach of running a script in travis that would fail the build if the PR wasn't the right format. Maybe somehow using the underlying CLI from commitizen to run against the Travis PR title.

This bot is interesting. Not quite sure how we'd run it in travis. The docs are sparse to say the least...

I think it might be worthwhile to move this discussion about PR title validation to a separate issue and leave this to be just around the Contributing guidelines

@BenAtEventbrite BenAtEventbrite added this to the MVP milestone Apr 5, 2018
@rwholey-eb
Copy link

rwholey-eb commented Apr 5, 2018

@BenAtEventbrite +1 on moving to a new issue, i think for this pr i'm going to add a CONTRIBUTING.md an issues template and a pr template (about halfway done) and we can work out the rest in that separate issue. sound good?

@benmvp
Copy link
Contributor

benmvp commented Apr 5, 2018

I know we're no longer talking about commit title validation in Travis, but I stumbled upon http://marionebl.github.io/commitlint/#/guides-ci-setup which may help us and I need somewhere to put it :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants