-
-
Notifications
You must be signed in to change notification settings - Fork 51
Contribution guidelines #18
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
Conversation
README is good, overall just had some minor questions |
Also another thing, should we discuss about deleting branches after a PR? |
Good question, I never delete branches but that's something I learned back at my previous company. I could do some research on the pros and cons |
@HappyZombies could you please add some suggestions to the lower sections, especially |
I actually suggest we hold off on that section for a bit (at least for in this PR) -- I think with you, me and jwerre, we can take the maintainer status from here on out. If you think we really need a document for this, let's look into making a MAINTAINERS.md file instead. That way it separates out the contributors and maintainers. I looked around and found some different ways of doing it. I will share a few examples below. https://github.com/zendframework/maintainers/blob/master/MAINTAINERS.md Different ways of doing it. If anything, remove the maintainer section and I will create a new PR that contains this MAINTAINERS.md file, that way we can review it more later. Watcha think? |
That's a good idea |
@jankapunkt some small typos, once those are address I will approve and merge! |
Indeed, too many branches will interfere with the work and it will be difficult to understand them, of course, it is better not to have them, on the other hand, it may be necessary to see something that was there if for some reason it was not in history, and for such a case you can old move branches to a separate repository. A PR was once made to one of my repositories, after which it was removed along with the repository and closed. I could not open or download the branch. However, all the changes on the PR site were still visible on GitHub. |
so @HappyZombies @oklas should we remove PRs for fixes and features but keep those for releases? I think that's what the old project did, too |
I think we should remove branches that are features and bug fixes, too many branches (at least to me) become kind of overwhelming and difficult to work. However we keep the ones for releases like you mentioned |
@jwerre is something you have to add to this? |
As far as have seen it is generic approach and I agree with that too |
I will wait for #32 to be merged into this and then we are good to go, I think |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jankapunkt sorry, I keep forgetting to leave this review cuz the changes I marked are pending.
Just small typos and then it's ready to go!
Add commit message convention and coding rules
Okay @HappyZombies @oklas @jwerre from my end it's done 🎉 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved! Hooray
This is just a draft for now as things are not 100% clear. Related discussion: #5