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

Updated "Decision Making" section to reflect current uncertainty #14

Merged
merged 1 commit into from
Jan 18, 2016

Conversation

taoeffect
Copy link

No description provided.

@jtoomim
Copy link

jtoomim commented Jan 17, 2016

Yes, we still have to decide how we make decisions.

I think that a lot of the stuff in there still applies, and could stay. For example, everything in the "In general, all pull requests must:" list seems like it's worth keeping.

###Peer Review

Anyone may participate in peer review which is expressed by comments in the pull request. Typically reviewers will review the code for obvious errors, as well as test out the patch set and opine on the technical merits of the patch. Project maintainers take into account the peer review when determining if there is consensus to merge a pull request (remember that discussions may have been spread out over github, mailing list and IRC discussions). The following language is used within pull-request comments:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you delete those lines?

Will there be no peer review?
How will decisions be made?

NACK ❌

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no idea what the decision making process actually is. It could be radically different from Bitcoin Core (unless otherwise stated) and since answer I got for #13 was "We're not sure yet" + "we have a draft here on this site" I figured at least don't include anything that could be potentially misleading. A "TBD" is not misleading.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for clarifying the unclarity!

@taoeffect
Copy link
Author

OK, so can someone be a bit clearer then on which parts are still true regarding the decision-making process and which aren't?

I get that the process regarding protocol changes are up in the air, so that can go. What is known that's still the same?

@jtoomim
Copy link

jtoomim commented Jan 17, 2016

@taoeffect Ignore orangeman, he has been going to all of our PRs and spreading FUD and generally trolling.

We need to write a bot that posts about him after each of his posts, maybe. Don't feed the troll.

@jtoomim
Copy link

jtoomim commented Jan 17, 2016

I get that the process regarding protocol changes are up in the air, so that can go. What is known that's still the same?

I'm hoping @toomim will come in and make some suggestions, or write a draft. You could too.

Basically, we want to keep the code quality high, and should be encouraging people to check their work and not submit crap. But we also want to be doing things in a democratic manner, which https://bitcoinclassic.consider.it/how-to-change-the-code?results=true describes. That's just a draft, but it's a good start, I think, and reflects my preferences and the preferences of a few other founding members. But it's still up for debate.

@ahmedbodi
Copy link

I agree with @jtoomim. Another thing to add is that code should ideally come with:

  • Unit tests if possible
  • Any research e.g. why X params were chosen
  • Use cases etc
  • Comments in the code (a MUST, the current code is lacking in numerous places)
    Simply because it helps in the decision making process and helps people get a better understanding of the purpose behind a PR and aids in the testing, usage and documenting of it.

@orangeman orangeman mentioned this pull request Jan 17, 2016
@taoeffect
Copy link
Author

OK, I don't want to waste your time guys, so I'm going to close this PR since I don't have the interest or time to write the Decision Making section (sorry) and I am afraid I would do a poor job of representing the consensus if I tried. Hopefully someone else can open another PR to take care of #13.

@taoeffect taoeffect closed this Jan 17, 2016
@jtoomim
Copy link

jtoomim commented Jan 17, 2016

Okay, thanks for trying @taoeffect. We'll figure it out soon.

@toomim
Copy link

toomim commented Jan 17, 2016

@taoeffect You don't have to represent the consensus. You can propose something on consider.it, and then people will tell you how much they support or oppose it. Then you can learn from that and improve the proposal. Finding consensus is an iterative process.

@toomim
Copy link

toomim commented Jan 17, 2016

@jtoomim Thanks for calling my attention to this request.

@taoeffect I actually like your pull request and would like to see it merged. Can we accept this request? Removing lines we don't like is a good first step to writing the ones we do like. I can then add back the lines that jonathan wants in a separate pull.

@taoeffect taoeffect reopened this Jan 18, 2016
@taoeffect
Copy link
Author

Per @toomim's request, re-opening.

@ahmedbodi
Copy link

Alright, I'll merge this then we can add another PR later to add the new information

ahmedbodi pushed a commit that referenced this pull request Jan 18, 2016
Updated "Decision Making" section to reflect current uncertainty
@ahmedbodi ahmedbodi merged commit e2cb32f into bitcoinclassic:master Jan 18, 2016
@jtoomim
Copy link

jtoomim commented Jan 18, 2016

@essofluffy on slack is working on a new version.

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

Successfully merging this pull request may close these issues.

5 participants