Improve Contribution Workflow

VulcanJS / Vulcan

🌋 A toolkit to quickly build apps with React, GraphQL & Meteor

http://vulcanjs.org

MIT License

7.98k stars 1.88k forks source link

Improve Contribution Workflow #1842

Open SachaG opened 6 years ago

SachaG commented 6 years ago

Discuss!

MHerszak commented 6 years ago

https://github.com/blog/2111-issue-and-pull-request-templates

justinr1234 commented 6 years ago

Require 2 contributors to sign off on a PR:

https://github.com/underarmour/further-review

Looks like it could be deployed to heroku for free via the docker image: https://devcenter.heroku.com/articles/container-registry-and-runtime

Combine the heroku with https://uptimerobot.com/ to prevent the instance from suspending on the free tier.

justinr1234 commented 6 years ago

CI to do the following on all PRs:

Circle CI and Travis both have free plans. It looks like Circle may be more performant. One thing to keep in mind is that builds could take a really long time if there's a lot of merges & prs.

Run tests (if we had them)
Run linting (very quick and easy)
(this takes longer) Run a meteor bundle against the intended versions of npm, node, and meteor including a fresh npm i to make sure everything runs properly
Run through a headless chrome as a smoke test (https://github.com/GoogleChrome/puppeteer)

Really if we could just verify a npm run lint I'd be pretty happy.

luhagel commented 6 years ago

Here is a gist of my current CircleCI setup, runs pretty well and builds only take 1-2 minutes, so even when prs start piling up, we should be good to go. Additionally, as an open source project, we get 3 free containers extra, which in theory allows 4 builds to run in parallel

SachaG commented 6 years ago

Some other guidelines:

All PRs should be made to the devel branch.
PRs to example packages should be made to the Starter repo.
PRs need the approval of two contributor team members or one core team member before being merged.
Make separate PRs for separate features.
Only include commits relevant to your PR.
Do not include formatting/cosmetic changes, they make the diff harder to read. If you want to fix formatting, make a specific, separate PR.
If a PR is merged to master by mistake, just leave it be until we can discuss.
All PR code should be commented as much as possible. Better too many comments than too few.
Non-obvious bug fixes should include code comments referencing the GitHub issue/Stack Overflow question/documentation item/etc. explaining the fix.
Any new setting or callback in the PR code should be registered.
Any string in the PR code should be internationalized.

justinr1234 commented 6 years ago

As part of the checklist for merging a PR, we should also see if it requires a documentation change.

MathiasKandelborg commented 6 years ago

Hi! I got a bit sad when I saw the contributing.md and found this issue that hopefully adresses it. I've found a template for creating a good CONTRIBUTING.md which serves as a great baseline.

stale[bot] commented 5 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.