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

Define Maintainer rules #119

Closed
chimp1984 opened this issue Sep 10, 2019 · 11 comments
Closed

Define Maintainer rules #119

chimp1984 opened this issue Sep 10, 2019 · 11 comments

Comments

@chimp1984
Copy link

I would suggest following rules for Bisq codebase maintainers which are partly derived from the way how it is handled in Bitcoin (to my knowledge, please correct if anything is wrong).

  • Official Bisq maintainers (currently @ripcurlx , @sqrrm and @freimair) play a special role when it comes to reviewing and merging pull requests. They have basically a veto right by putting a NACK to a PR and then no maintainer must merge that PR. It will require a follow up utACK or ACK from the same reviewer before it can get merged.
  • The review results of other developers should also be considered for the merge decision. They can be weighted by the devleopers contribution history (e.g. a contibutor who is already experienced and delivered valuable work in the past will have higher weight than a fresh dev). Any NACK should be taken seriously but of course we don't want to get in a deadlock by getting blocked by NACKs from non-maintainers.
  • Any code change in the DAO package require a ACK/utACK from @ManfredKarrer. It is consensus code and carries high risk even with small changes which might look trival to other developers.
  • Adding a dependency or version update need a proposal with clear argumentation why we need that change. We need to be very conservative with adding/changing dependencies and should try to reduce dependencies rather to add new ones. Only if the proposal gets clear support it can be merged. Removing a dependency is always welcome.
  • For critical and more complex PRs the core maintainers should give enough time to receive reviews from other maintainers. Best to have at least 2 ACKs for any non trivial PR.
  • If there is no consensus found the default policy is "no change".
  • A new developer should to be active in Bisq for at least 3 months before he can be considered to become maintainer. The process is to make a bonded role request in the DAO, and after accepted by voting to set up the bond. After that the Github admin adds the dev as maintainer.
  • Beside @ripcurlx none of the existing maintainers has set up the bond yet. This was ok, as we did not enforce encourage and bonding so far. We should define a timeline until when the existing maintainers need to make their bond as well. We could also require the voting process (no strong opinion on that, but would make the process more DAO conform).

Please add your opinon about the suggestions.
If you support that proposal please upvote, otherwise downvote.

@wiz
Copy link

wiz commented Sep 10, 2019

I actually proposed to make bisq a "protected branch" in GitHub to enforce this policy a few days back https://help.github.com/en/articles/about-protected-branches

@chimp1984
Copy link
Author

@wiz Good idea. I am not much familiar with the details but after a quick look it seems it supports what we want.

@chimp1984
Copy link
Author

FYI re DAO complexity:
bisq-network/bisq#3241 (comment)

@chimp1984
Copy link
Author

@ripcurlx , @sqrrm , @freimair What is your opinion about that proposal?

@ripcurlx
Copy link

@ripcurlx , @sqrrm , @freimair What is your opinion about that proposal?

ACK ;-)

@sqrrm
Copy link
Member

sqrrm commented Sep 19, 2019

ACK

@wiz
Copy link

wiz commented Sep 20, 2019

Seems like we have consensus - @ripcurlx are you going to implement this proposal by setting up the "protected branch" feature of GitHub for Bisq and other critical repositories ?

@chimp1984
Copy link
Author

@freimair can you add your opinion?

@freimair
Copy link

ACK

Keep in mind: no-change might not always be the best option. I would decide that in context.

Furthermore, I do not have the required amount of BSQ available to set up the maintainer bond.

@erciccione
Copy link

Any code change in the DAO package require a ACK/utACK from @ManfredKarrer. It is consensus code and carries high risk even with small changes which might look trival to other developers.

Isn't this a bottleneck? Woudn't be better to require at least two ACKs from core maintainers? Would also be a more decentralized solution than requiring an approval from one specific person (the approval is required for one person, not role or figure, which is a very centralized solution).

@chimp1984
Copy link
Author

Close as it got accepted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants