{"action":"opened","issue":{"url":"https://api.github.com/repos/alatyami/Growthcraft-1.7/issues/34","labels_url":"https://api.github.com/repos/alatyami/Growthcraft-1.7/issues/34/labels{/name}","comments_url":"https://api.github.com/repos/alatyami/Growthcraft-1.7/issues/34/comments","events_url":"https://api.github.com/repos/alatyami/Growthcraft-1.7/issues/34/events","html_url":"https://github.com/alatyami/Growthcraft-1.7/issues/34","id":100443537,"number":34,"title":"Review Process","user":{"login":"IceDragon200","id":1649873,"avatar_url":"https://avatars.githubusercontent.com/u/1649873?v=3","gravatar_id":"","url":"https://api.github.com/users/IceDragon200","html_url":"https://github.com/IceDragon200","followers_url":"https://api.github.com/users/IceDragon200/followers","following_url":"https://api.github.com/users/IceDragon200/following{/other_user}","gists_url":"https://api.github.com/users/IceDragon200/gists{/gist_id}","starred_url":"https://api.github.com/users/IceDragon200/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/IceDragon200/subscriptions","organizations_url":"https://api.github.com/users/IceDragon200/orgs","repos_url":"https://api.github.com/users/IceDragon200/repos","events_url":"https://api.github.com/users/IceDragon200/events{/privacy}","received_events_url":"https://api.github.com/users/IceDragon200/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":0,"created_at":"2015-08-12T00:52:46Z","updated_at":"2015-08-12T00:52:46Z","closed_at":null,"body":"After the advent of #31, #32 and #33, I think I need to raise this issue so this problem never happens again. Sorry if I'm picking on anyone, but its for the better.\r\n\r\nBasically, what I've been introduced to is the concept of maintainer check offs, where you create PR, the maintainers review it, and then a number of the maintainers give a `LGTM`, \"Looks Good To Me\", that's when its merged, so PRs are allowed to sit around until a portion of them give the go ahead to merge, therefore the master branch is considered sacred! Thou shall not rebase master, and thou shall not force push over master! This is what feature branches are for, before the branch is merged the maintainers can ask for the commits to be `squashed` to reduce commit footprints, especially if the commits are riddled with small patches/fixups for a feature being added.\r\n\r\nSo the usual routine is:\r\n```\r\nCreate feature branch\r\nDo some work and push commits\r\nGet feature somewhat working\r\nOpen PR\r\nMaintainers review and point out things that can be changed/removed/added\r\nGo back do some more work, push commits\r\nMaintainers review again, possibly giving the LGTM, and ask for a squash\r\nGo back squash commits and push\r\nNow everyone waits for tests to pass\r\nONWARDS TO MERGE CITY\r\n...\r\nPROFIT!?\r\n```\r\n\r\nAnd then you repeat this for every feature, this ensures a level of master branch stability.\r\n\r\nI also apologize for nudging the version number (the `2.2.0` release :package: : and `2.2.1` patch :hammer:), that should be done by the maintainers as they deem fit.\r\n\r\nAnyway:\r\ntl;dr\r\nWhen a Pull Request is created, slow down on the merge finger, review it, call in a few more maintainers or community members to review the changes, remember this affects all of us, we should all have input it what goes in and what needs fixing. In short don't be afraid to voice your opinion on a feature.\r\n\r\n\r\nHere ends my rant :octocat: \r\n\r\n@alatyami \r\n@Sunconure11 "}}