[NC-1540] First Draft of OSS Roles & Std OSS Documenation #29

ekellstrand · 2018-10-10T16:46:49Z

I expect there will be some good debate over the contents. I think major discussions should be taken offline and logged as separate tickets to solve and update.

FYI: There are some TODO's left before we ship. I will create tickets for anything that is still outstanding when the PR is merged.

Added Templates

smatthewenglish · 2018-10-10T17:48:36Z

when I was commiting to goethe and bitcoind I always had a kind of "champion", someone with write access who I discussed the proposed change with first, and they validated that it was something sensible, personally I think it would be a good thing to make that process, which I guess is pretty common to open source projects, to make it explicit, so that you have some kind of checkbox for a benefactor. what do you guys think of that?

.github/issue_template.md

.github/pull_request_template.md

NicolasMassart · 2018-10-11T09:16:12Z

.github/issue_template.md

+* [ ] Put an X between the brackets on this line if you have done all of the following:
+  * Reproduced the issue in the latest version of the software
+  * Followed all steps in the debugging wiki: https://github.com/PegasysEng/pantheon/wiki/debugging
+  * Verified the issue doesn't already exist: https://github.com/search?q=+is%3Aissue+repo%3AConsenSys/Pantheon


The link points to the old repos, should be https://github.com/search?q=+is%3Aissue+repo%3APegaSysEng/Pantheon

NicolasMassart · 2018-10-11T09:34:55Z

CONTRIBUTING.md

+#### Before Submitting An Enhancement Suggestion
+
+* **Check the [Debugging Wiki].** You might be able to find the cause of the problem and fix things yourself. 
+* **Perform a [cursory search of project issues](https://github.com/search?q=+is%3Aissue+repo%3AConsenSys/Pantheon)** to see if the problem has already been reported. If it has **and the issue is still open**, add a comment to the existing issue instead of opening a new one.


same here, should be https://github.com/search?q=+is%3Aissue+repo%3APegaSysEng/Pantheon

CONTRIBUTING.md

NicolasMassart · 2018-10-11T10:09:23Z

CONTRIBUTING.md


-The test cases are sufficient enough to provide confidence in the code’s robustness, while avoiding redundant tests.
+Please open an issue on `ConsenSys/Pantheon` if you have suggestions for new labels.


Should be PegaSysEng/pantheon

NicolasMassart · 2018-10-11T10:12:05Z

CONTRIBUTING.md


-The code is free from glaring typos (*e.g. misspelled comments*), thinkos, or formatting issues (*e.g. incorrect indentation*).
+[search-label-enhancement]: https://github.com/search?q=is%3Aopen+is%3Aissue+repo%3AConsenSys%2FPantheon+label%3Aenhancement


Same here for all links, should be PegaSysEng instead of ConsenSys

Updated for migration from Consensys to PegasysEng repo

# Conflicts: # README.md

GOVERNANCE.md

README.md

mbaxter · 2018-10-11T17:50:56Z

.github/pull_request_template.md

+- [ ] I have performed a self-review of my own code
+- [ ] I have commented my code, particularly in hard-to-understand areas
+- [ ] I have made corresponding changes to the documentation
+- [ ] My changes generate no new warnings


Do we need this? Seems like something we should enforce in CI if we want this.

Possibly. Same response as below in the PR template. I'm inclined to leave warnings line in (for now) because I think a recent change no longer fails on warning. eg, I get a warning on builds locally that doesn't fail it.

mbaxter · 2018-10-11T17:53:02Z

.github/pull_request_template.md

+
+```
+
+## Prerequisites:


I wonder if we need this in every PR. Might make more sense as part of contributing.md.

It's boiler plate. And I suspect we'll want to make changes to this. I'd like to log separate issues to address content changes like this. We can discuss the merits of the change in the issue. It will also be useful to land this and see what it actually looks like.

FYI: It does seem pretty common on other projects. It's obviously aimed at a big external community, and making sure everyone understands the rules, whether they grok the CONTRIBUTING.md or not.

shemnon

Can we get a markdown formatter run against this? There are at least three different column length models I see (80, 100, and none) and that makes it harder to read.

shemnon · 2018-10-11T17:49:31Z

docs/community/community-membership.md

+
+Members are continuously active contributors in the community.  They can have
+issues and PRs assigned to them.
+


Should we add a "defined by" section and have it be that they are a member of a particular github group?

That's one way. file contents are another common way. Wiki page, etc. We won't solve it in this PR. I'm going to add issues for all of the outstanding TODOs that are placeholders.

shemnon · 2018-10-11T17:50:39Z

docs/community/community-membership.md

+- Approval of new users to the Approver role, and access to Circle reports.
+
+## Open Source Circle
+The Open Source Circle is a group provides open source software support to projects.


is a group that provides

shemnon · 2018-10-11T17:56:29Z

docs/development/logging.md

+
+```
+ALL:	All levels including custom levels.
+DEBUG:	Designates fine-grained informational events that are most useful to debug an application.


These should be in order of less to more: OFF, FATAL, ERROR, WARN, INFO, DEBUG, TRACE, ALL.

ekellstrand · 2018-10-11T18:47:20Z

@shemnon Added NC-1695 for Markdown formatting with Spotless
NC-1630 Roadmap doc
NC-1632 Architecture Doc
NC-1697 Fill in objectives.md
NC-1698 Fill in design-principles.md
NC-1696 community.md role membership definition & doc TODOs
NC-1699 Fill in download badge links once artifacts exist on bintray
NC-1700 Fill in documentation style guide in CONTRIBUTING.md
NC-1633 Talk to Legal to implement a CLA process

fixed some typos and organization suggestions from code review

shemnon

LTGM given that all my other concerns are pushed into other JIRAs.

MadelineMurray · 2018-10-11T20:10:27Z

.github/issue_template.md

+
+* [ ] Put an X between the brackets on this line if you have done all of the following:
+  * Reproduced the issue in the latest version of the software
+  * Followed all steps in the debugging wiki: https://github.com/PegasysEng/pantheon/wiki/debugging


This doesn't exist yet and I'm not sure it's covered by an exisiting Jira ticket? Should it belong with the other contributing docs in the /docs folder rather than the Wiki? Does this content exist anywhere yet?

I created an empty wiki page for debugging, so the link does work - it's just not useful yet. NC-1703 is created to address the content and/or move it.

MadelineMurray · 2018-10-11T20:14:34Z

CONTRIBUTING.md

+  * [Suggesting Enhancements](#suggesting-enhancements)
+  * [Your First Code Contribution](#your-first-code-contribution)
+  * [Pull Requests](#pull-requests)
+  * [Code Reviews]


Should this link somewhere?

yes. I'll fix that in follow-up PR

MadelineMurray · 2018-10-11T20:20:12Z

CONTRIBUTING.md

+    * :shirt: `:shirt:` when removing lint warnings
+
+## Documentation Style Guide
+**TODO: Create Documentation Style Guide**


We have one of these - if you assign the Jira ticket to the Doc team, we can move it over from GDrive to GitHub.

NC-1700 covers this.

MadelineMurray

See specific comments.

macfarla · 2018-10-12T06:42:20Z

README.md

@@ -1,4 +1,9 @@
-# Pantheon Ethereum Client &middot; [![License](https://img.shields.io/badge/License-Apache%202.0-blue.svg)](LICENSE)
+# Pantheon Ethereum Client
+ [![Build Status](http://forge-jenkins.kellstrand.com:8080/job/Pantheon/job/master/badge/icon)](http://forge-jenkins.kellstrand.com:8080/job/Pantheon/job/master/)


There's a new link for the build status badge

macfarla · 2018-10-12T06:42:58Z

README.md

@@ -19,104 +24,17 @@ User and reference documentation available on the Wiki includes:

 ## Pantheon Developers

-## Build Instructions
+* [Contribution Guidelines](CONTRIBUTING.md)
+* [Wiki](wiki/) for installation & configuring Pantheon


installation & configuration

macfarla · 2018-10-12T06:43:58Z

docs/community/code-reviews.md

+reviewers. The primary reviewer will make this decision and nominate a second
+reviewer, if needed. Except for trivial changes, PRs should not be committed
+until relevant parties (e.g. owners of the subsystem affected by the PR) have
+had a reasonable chance to look at PR in their local business hours.


macfarla · 2018-10-12T06:44:45Z

docs/community/code-reviews.md

+
+Except for rare cases, such as trivial changes (e.g. typos, comments) or
+emergencies (e.g. broken builds), approvers should not merge their own
+changes without another approver providing the code review.


do we even allow merging without approving?

macfarla · 2018-10-12T06:54:23Z

docs/community/community-membership.md

+| -----| ---------------- | ---------- |
+| Everyone | none | anybody with a belly button
+| Member | everyone who contributes - code or otherwise | Pantheon GitHub org member
+| Approver | approve accepting contributions | write permissions on master


s/accepting/accepted ?

jimjag · 2018-10-12T11:21:21Z

.github/issue_template.md

@@ -0,0 +1,40 @@
+<!--


Although a BDFL is "self appointed", they are also someone who has been bequeathed that power by the community. Think of Monty Python and the Holy Grail: "Supreme Executive power derives from a mandate from the masses." So yeah, for example, Linus is BDFL for Linux because he appointed himself as such, but also because the community is OK with him being so. This is a subtle be important point. If someone just self-appoints and the community is not OK w/ it, the project will fail.

jimjag · 2018-10-12T11:29:48Z

.github/issue_template.md

@@ -0,0 +1,40 @@
+<!--


I think the workflow section likely needs more meat. As an example, look at https://kafka.apache.org/contributing , which breaks out code and doc contributions. Also, note that there exists https://cwiki.apache.org/confluence/display/KAFKA/Contributing+Code+Changes which goes into more detail about the actual workflow (we should not just copy/paste this... IMO, the Kafka guidelines are too wordy and too onerous for people to follow... something more streamlined is best)

ekellstrand added 2 commits October 10, 2018 12:41

[NC-1540] First Draft of OSS Roles & Std OSS Documentation

f11ca7e

[NC-1540] First Draft of OSS Roles & Std OSS Documentation

9926b7d

Added Templates

ekellstrand requested review from NicolasMassart, mbaxter, ajsutton, bretthenderson and MadelineMurray October 10, 2018 16:46

ekellstrand commented Oct 10, 2018

View reviewed changes

.github/issue_template.md Show resolved Hide resolved

ekellstrand commented Oct 10, 2018

View reviewed changes

.github/pull_request_template.md Show resolved Hide resolved

NicolasMassart reviewed Oct 11, 2018

View reviewed changes

NicolasMassart requested a review from donntrenton October 11, 2018 10:34

ekellstrand added 3 commits October 11, 2018 11:53

[NC-1540] First Draft of OSS Roles & Std OSS Documentation

c47fc9f

Updated for migration from Consensys to PegasysEng repo

Merge remote-tracking branch 'upstream/master' into NC-1540

50c7d1c

# Conflicts: # README.md

Renamed documentation to docs for better tool integration

209ba82

mbaxter reviewed Oct 11, 2018

View reviewed changes

shemnon reviewed Oct 11, 2018

View reviewed changes

[NC-1540] First Draft of OSS Roles & Std OSS Documentation

f1784c0

fixed some typos and organization suggestions from code review

shemnon approved these changes Oct 11, 2018

View reviewed changes

Merge branch 'master' into NC-1540

b2b9e45

MadelineMurray reviewed Oct 11, 2018

View reviewed changes

Merge branch 'master' into NC-1540

bf5c385

MadelineMurray reviewed Oct 11, 2018

View reviewed changes

ekellstrand merged commit 23fa4ca into PegaSysEng:master Oct 11, 2018

ekellstrand deleted the NC-1540 branch October 11, 2018 22:05

macfarla reviewed Oct 12, 2018

View reviewed changes

jimjag reviewed Oct 12, 2018

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[NC-1540] First Draft of OSS Roles & Std OSS Documenation #29

[NC-1540] First Draft of OSS Roles & Std OSS Documenation #29

ekellstrand commented Oct 10, 2018

smatthewenglish commented Oct 10, 2018

NicolasMassart Oct 11, 2018

NicolasMassart Oct 11, 2018

NicolasMassart Oct 11, 2018

NicolasMassart Oct 11, 2018

mbaxter Oct 11, 2018

ekellstrand Oct 11, 2018

mbaxter Oct 11, 2018

ekellstrand Oct 11, 2018

ekellstrand Oct 11, 2018

shemnon left a comment

shemnon Oct 11, 2018

ekellstrand Oct 11, 2018

shemnon Oct 11, 2018

shemnon Oct 11, 2018

ekellstrand commented Oct 11, 2018 •

edited

Loading

shemnon left a comment

MadelineMurray Oct 11, 2018

ekellstrand Oct 11, 2018

MadelineMurray Oct 11, 2018

ekellstrand Oct 11, 2018

MadelineMurray Oct 11, 2018

ekellstrand Oct 11, 2018

MadelineMurray left a comment

macfarla Oct 12, 2018

macfarla Oct 12, 2018

macfarla Oct 12, 2018

macfarla Oct 12, 2018

macfarla Oct 12, 2018

jimjag Oct 12, 2018

jimjag Oct 12, 2018


		The test cases are sufficient enough to provide confidence in the code’s robustness, while avoiding redundant tests.
		Please open an issue on `ConsenSys/Pantheon` if you have suggestions for new labels.


		The code is free from glaring typos (e.g. misspelled comments), thinkos, or formatting issues (e.g. incorrect indentation).
		[search-label-enhancement]: https://github.com/search?q=is%3Aopen+is%3Aissue+repo%3AConsenSys%2FPantheon+label%3Aenhancement


		Members are continuously active contributors in the community. They can have
		issues and PRs assigned to them.


		```

		## Prerequisites:

[NC-1540] First Draft of OSS Roles & Std OSS Documenation #29

[NC-1540] First Draft of OSS Roles & Std OSS Documenation #29

Conversation

ekellstrand commented Oct 10, 2018

smatthewenglish commented Oct 10, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shemnon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ekellstrand commented Oct 11, 2018 • edited Loading

shemnon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MadelineMurray left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ekellstrand commented Oct 11, 2018 •

edited

Loading