iamtrask / DEPRECATED

DEPRECATED - do not use!!!
Apache License 2.0
349 stars 48 forks source link

Feature/contributing #16

Closed kevinahuber closed 6 years ago

kevinahuber commented 6 years ago

Contributing!

This is a start for our contributing guidelines.

We should keep the root of docs relatively clean, with the readme directing people to the appropriate place.

I intend on adding a reviewing section for guidelines of our reviewers with things like when to squash, responding guideline, etc.

This addresses #9

NOTE: I moved the quickstart guide (contributing-quickstart.md -> contributing/quickstart.md) This will break some links we have already been distributing

samsontmr commented 6 years ago

Some sections are covered by my PR #9. Should I close mine? Or we can link to the other markdowns instead of duplicating the sections.

kevinahuber commented 6 years ago

Yep! Haha my redundancy of your work just proves the need for it :)

I'll pull your concepts into mine, then if you can review it afterwards to confirm that I haven't kept anything out.

kevinahuber commented 6 years ago

@samsontmr Take a look

adrianbrink commented 6 years ago

I would recommend that instead of developing our own convention for branching and merging we should adopt git-flow.

It is a very simple yet very effective branching strategy that allows a lot of people to contribute.

As an extensions I would suggest that everyone has to include their own name in the branch name. Furthermore I would suggest that we every PR has to be tagged with 'bug', 'feature', 'docs', ... .

An example:

I want to fix a bug a discovered on develop.

I checkout my branch from develop using git checkout -b feature/adrian-bug_fix.

I do some development, I commit it, and then push that branch to the main repository.

Now I open a PR from that branch onto develop and tag it with bug.

Unresolved questions: How will this flow work from forks?

anoff commented 6 years ago

Do we need develop and master? In my experience we often dropped one of the branches because they were only there to satisfy a process. Never had an actual use case for it.

I'm favouring gitlab flow, read the link for some strong arguments :)

adrianbrink commented 6 years ago

We are currently using it for Tendermint and Ethermint and we do all the work on develop and then merge it into master. No one ever touches master directly and we so far it has prevented us from broken code on master.

anoff commented 6 years ago

What would happen if master breaks? In my experience: a pipeline fails, code does net get deployed, some alerts go off and people fix it. Same thing happens if you break develop. If you are working with short iterations issues should get fixed (fail forward).

The question is if anyone has ever had bad experiences with not having a develop branch. Otherwise I don't see a reason to choose the complex workflow over the simple one?

iamtrask commented 6 years ago

@cereallarceny anything more to add prior to a merge? (also... we still have some conflicts to address)

cereallarceny commented 6 years ago

None for me in particular @iamtrask - I've mostly just been following along for the ride. Personally, I've always loved git-flow. That's my preference for workflow since it's pretty universally used in every major project I've ever done. I think the biggest argument to use git-flow is actually just because it's so commonly known and would be most intuitive for new contributors. There's no need to learn another Github contribution model when there's already a great one established. However, don't let that stop us from defining our own system.

Maybe I also misunderstand the argument here. That's fairly likely. :)

kevinahuber commented 6 years ago

Up for gitflow + branch naming. Excellent feedback. So

kevinahuber commented 6 years ago

@adrianbrink

As an extensions I would suggest that everyone has to include their own name in the branch name.

I don't agree with this- it would be redundant if the user is PR'ing off of a fork, and it gets gnarly if there is more than one person working on a feature. What are your thoughts behind it?

kevinahuber commented 6 years ago

Olright! I think I've addressed everything. If you have things to add, but agree with this general framework, please file an issue for a future PR.

Two things to do once this is merged:

iamtrask commented 6 years ago

@cereallarceny love to get your review on this ticket before we merge :)