update CONTRIBUTING.md and DEVELOPMENT_WORKFLOW.md (#1475)

* update CONTRIBUTING.md and DEVELOPMENT_WORKFLOW.md * Update contrib/DEVELOPMENT_WORKFLOW.md Co-authored-by: philanthrope <[email protected]> * move styles to style guide * update workflow doc based on comment * update contributing.md delete Architecture section * fix CODE_REVIEW_DOC dev_note link * Update contrib/CONTRIBUTING.md Great Co-authored-by: philanthrope <[email protected]> * Update contrib/CONTRIBUTING.md Ditto: Co-authored-by: philanthrope <[email protected]> --------- Co-authored-by: gitphantomman <[email protected]> Co-authored-by: philanthrope <[email protected]>
opentensor · Aug 16, 2023 · 3fefdbb · 3fefdbb
1 parent 2ee07a0
commit 3fefdbb
Show file tree

Hide file tree

Showing 3 changed files with 171 additions and 5 deletions.
diff --git a/contrib/CODE_REVIEW_DOCS.md b/contrib/CODE_REVIEW_DOCS.md
@@ -0,0 +1,71 @@
+# Code Review
+### Conceptual Review
+
+A review can be a conceptual review, where the reviewer leaves a comment
+ * `Concept (N)ACK`, meaning "I do (not) agree with the general goal of this pull
+   request",
+ * `Approach (N)ACK`, meaning `Concept ACK`, but "I do (not) agree with the
+   approach of this change".
+
+A `NACK` needs to include a rationale why the change is not worthwhile.
+NACKs without accompanying reasoning may be disregarded.
+After conceptual agreement on the change, code review can be provided. A review
+begins with `ACK BRANCH_COMMIT`, where `BRANCH_COMMIT` is the top of the PR
+branch, followed by a description of how the reviewer did the review. The
+following language is used within pull request comments:
+
+  - "I have tested the code", involving change-specific manual testing in
+    addition to running the unit, functional, or fuzz tests, and in case it is
+    not obvious how the manual testing was done, it should be described;
+  - "I have not tested the code, but I have reviewed it and it looks
+    OK, I agree it can be merged";
+  - A "nit" refers to a trivial, often non-blocking issue.
+### Code Review
+Project maintainers reserve the right to weigh the opinions of peer reviewers
+using common sense judgement and may also weigh based on merit. Reviewers that
+have demonstrated a deeper commitment and understanding of the project over time
+or who have clear domain expertise may naturally have more weight, as one would
+expect in all walks of life.
+
+Where a patch set affects consensus-critical code, the bar will be much
+higher in terms of discussion and peer review requirements, keeping in mind that
+mistakes could be very costly to the wider community. This includes refactoring
+of consensus-critical code.
+
+Where a patch set proposes to change the Bittensor consensus, it must have been
+discussed extensively on the mailing list and IRC, be accompanied by a widely
+discussed BIP and have a generally widely perceived technical consensus of being
+a worthwhile change based on the judgement of the maintainers.
+
+### Finding Reviewers
+
+As most reviewers are themselves developers with their own projects, the review
+process can be quite lengthy, and some amount of patience is required. If you find
+that you've been waiting for a pull request to be given attention for several
+months, there may be a number of reasons for this, some of which you can do something
+about:
+
+  - It may be because of a feature freeze due to an upcoming release. During this time,
+    only bug fixes are taken into consideration. If your pull request is a new feature,
+    it will not be prioritized until after the release. Wait for the release.
+  - It may be because the changes you are suggesting do not appeal to people. Rather than
+    nits and critique, which require effort and means they care enough to spend time on your
+    contribution, thundering silence is a good sign of widespread (mild) dislike of a given change
+    (because people don't assume *others* won't actually like the proposal). Don't take
+    that personally, though! Instead, take another critical look at what you are suggesting
+    and see if it: changes too much, is too broad, doesn't adhere to the
+    [developer notes](DEVELOPMENT_WORKFLOW.md), is dangerous or insecure, is messily written, etc.
+    Identify and address any of the issues you find. Then ask e.g. on IRC if someone could give
+    their opinion on the concept itself.
+  - It may be because your code is too complex for all but a few people, and those people
+    may not have realized your pull request even exists. A great way to find people who
+    are qualified and care about the code you are touching is the
+    [Git Blame feature](https://docs.github.com/en/github/managing-files-in-a-repository/managing-files-on-github/tracking-changes-in-a-file). Simply
+    look up who last modified the code you are changing and see if you can find
+    them and give them a nudge. Don't be incessant about the nudging, though.
+  - Finally, if all else fails, ask on IRC or elsewhere for someone to give your pull request
+    a look. If you think you've been waiting for an unreasonably long time (say,
+    more than a month) for no particular reason (a few lines changed, etc.),
+    this is totally fine. Try to return the favor when someone else is asking
+    for feedback on their code, and the universe balances out.
+  - Remember that the best thing you can do while waiting is give review to others!
diff --git a/contrib/CONTRIBUTING.md b/contrib/CONTRIBUTING.md
@@ -3,9 +3,11 @@
 The following is a set of guidelines for contributing to Bittensor, which are hosted in the [Opentensor Organization](https://github.com/opentensor) on GitHub. These are mostly guidelines, not rules. Use your best judgment, and feel free to propose changes to this document in a pull request.
 
 ## Table Of Contents
-
 1. [I don't want to read this whole thing, I just have a question!!!](#i-dont-want-to-read-this-whole-thing-i-just-have-a-question)
 1. [What should I know before I get started?](#what-should-i-know-before-i-get-started)
+1. [Getting Started](#getting-started)
+   1. [Good First Issue Label](#good-first-issue-label)
+   1. [Beginner and Help-wanted Issues Label](#beginner-and-help-wanted-issues-label)
 1. [How Can I Contribute?](#how-can-i-contribute)
    1. [Code Contribution General Guideline](#code-contribution-general-guidelines)
    1. [Pull Request Philosophy](#pull-request-philosophy)
@@ -14,6 +16,7 @@ The following is a set of guidelines for contributing to Bittensor, which are ho
    1. [Addressing Feedback](#addressing-feedback)
    1. [Squashing Commits](#squashing-commits)
    1. [Refactoring](#refactoring)
+   1. [Peer Review](#peer-review)
  1. [Reporting Bugs](#reporting-bugs)
  1. [Suggesting Features](#suggesting-enhancements)
 
@@ -34,6 +37,34 @@ Additionally, note that the core implementation of Bittensor consists of two sep
 
 Supplemental, yet necessary repositories are [openvalidators](https://github.com/opentensor/validators) and [openminers](https://github.com/opentensor/miners) which contain Bittensor Validators and Miners (respectively) designed by the OpenTensor Foundation team and open-sourced for the community to use.
 
+## Getting Started
+New contributors are very welcome and needed.
+Reviewing and testing is highly valued and the most effective way you can contribute as a new contributor. It also will teach you much more about the code and process than opening pull requests. 
+
+Before you start contributing, familiarize yourself with the Bittensor Core build system and tests. Refer to the documentation in the repository on how to build Bittensor core and how to run the unit tests, functional tests.
+
+There are many open issues of varying difficulty waiting to be fixed. If you're looking for somewhere to start contributing, check out the [good first issue](https://github.com/opentensor/bittensor/labels/good%20first%20issue) list or changes that are up for grabs. Some of them might no longer be applicable. So if you are interested, but unsure, you might want to leave a comment on the issue first.
+### Good First Issue Label
+The purpose of the good first issue label is to highlight which issues are suitable for a new contributor without a deep understanding of the codebase.
+
+However, good first issues can be solved by anyone. If they remain unsolved for a longer time, a frequent contributor might address them.
+
+You do not need to request permission to start working on an issue. However, you are encouraged to leave a comment if you are planning to work on it. This will help other contributors monitor which issues are actively being addressed and is also an effective way to request assistance if and when you need it.
+### Beginner and Help-wanted Issues Label
+You can start by looking through these `beginner` and `help-wanted` issues:
+
+* [Beginner issues](https://github.com/opentensor/bittensor/labels/beginner) - issues which should only require a few lines of code, and a test or two.
+* [Help wanted issues](https://github.com/opentensor/bittensor/labels/help%20wanted) - issues which should be a bit more involved than `beginner` issues.
+
+
+## Communication Channels
+Most communication about Bittensor development happens on Discord channel.
+Here's the link of Discord community.
+[Bittensor Discord](https://discord.com/channels/799672011265015819/799672011814862902)
+
+And also here.
+[Bittensor Community Discord](https://discord.com/channels/1120750674595024897/1120799375703162950)
+
 
 ## How Can I Contribute?
 
@@ -183,6 +214,21 @@ Project maintainers aim for a quick turnaround on refactoring pull requests, so
 
 Pull requests that refactor the code should not be made by new contributors. It requires a certain level of experience to know where the code belongs to and to understand the full ramification (including rebase effort of open pull requests). Trivial pull requests or pull requests that refactor the code with no clear benefits may be immediately closed by the maintainers to reduce unnecessary workload on reviewing.
 
+#### 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 taken place elsewhere, not just on GitHub). The following language is used within pull-request comments:
+
+- ACK means "I have tested the code and I agree it should be merged";
+- NACK means "I disagree this should be merged", and must be accompanied by sound technical justification. NACKs without accompanying reasoning may be disregarded;
+- utACK means "I have not tested the code, but I have reviewed it and it looks OK, I agree it can be merged";
+- Concept ACK means "I agree in the general principle of this pull request";
+- Nit refers to trivial, often non-blocking issues.
+
+Reviewers should include the commit(s) they have reviewed in their comments. This can be done by copying the commit SHA1 hash.
+
+A pull request that changes consensus-critical code is considerably more involved than a pull request that adds a feature to the wallet, for example. Such patches must be reviewed and thoroughly tested by several reviewers who are knowledgeable about the changed subsystems. Where new features are proposed, it is helpful for reviewers to try out the patch set on a test network and indicate that they have done so in their review. Project maintainers will take this into consideration when merging changes.
+
+For a more detailed description of the review process, see the [Code Review Guidelines](CODE_REVIEW_DOCS.md).
 
 ### Reporting Bugs
 

diff --git a/contrib/STYLE.md b/contrib/STYLE.md
@@ -6,21 +6,58 @@ Most programming languages have well-established conventions as to what constitu
 
 # Table of Contents
 1. [Code Style](#code-style)
-2. [Git Commit Style](#git-commit-style)
-3. [The Six Rules of a Great Commit](#the-six-rules-of-a-great-commit)
+2. [Naming Conventions](#naming-conventions)
+3. [Git Commit Style](#git-commit-style)
+4. [The Six Rules of a Great Commit](#the-six-rules-of-a-great-commit)
    - [1. Atomic Commits](#1-atomic-commits)
    - [2. Separate Subject from Body with a Blank Line](#2-separate-subject-from-body-with-a-blank-line)
    - [3. Limit the Subject Line to 50 Characters](#3-limit-the-subject-line-to-50-characters)
    - [4. Use the Imperative Mood in the Subject Line](#4-use-the-imperative-mood-in-the-subject-line)
    - [5. Wrap the Body at 72 Characters](#5-wrap-the-body-at-72-characters)
    - [6. Use the Body to Explain What and Why vs. How](#6-use-the-body-to-explain-what-and-why-vs-how)
-4. [Tools Worth Mentioning](#tools-worth-mentioning)
+5. [Tools Worth Mentioning](#tools-worth-mentioning)
    - [Using `--fixup`](#using---fixup)
    - [Interactive Rebase](#interactive-rebase)
-5. [Pull Request and Squashing Commits Caveats](#pull-request-and-squashing-commits-caveats)
+6. [Pull Request and Squashing Commits Caveats](#pull-request-and-squashing-commits-caveats)
 
 
 ### Code style
+
+#### General Style
+Python's official style guide is PEP 8, which provides conventions for writing code for the main Python distribution. Here are some key points:
+
+- `Indentation:` Use 4 spaces per indentation level.
+
+- `Line Length:` Limit all lines to a maximum of 79 characters.
+
+- `Blank Lines:` Surround top-level function and class definitions with two blank lines. Method definitions inside a class are surrounded by a single blank line.
+
+- `Imports:` Imports should usually be on separate lines and should be grouped in the following order:
+
+    - Standard library imports.
+    - Related third party imports.
+    - Local application/library specific imports.
+- `Whitespace:` Avoid extraneous whitespace in the following situations:
+
+    - Immediately inside parentheses, brackets or braces.
+    - Immediately before a comma, semicolon, or colon.
+    - Immediately before the open parenthesis that starts the argument list of a function call.
+- `Comments:` Comments should be complete sentences and should be used to clarify code and are not a substitute for poorly written code.
+
+#### For Python
+
+- `List Comprehensions:` Use list comprehensions for concise and readable creation of lists.
+
+- `Generators:` Use generators when dealing with large amounts of data to save memory.
+
+- `Context Managers:` Use context managers (with statement) for resource management.
+
+- `String Formatting:` Use f-strings for formatting strings in Python 3.6 and above.
+
+- `Error Handling:` Use exceptions for error handling whenever possible.
+
+#### More details
+
 Use `black` to format your python code before commiting for consistency across such a large pool of contributors. Black's code [style](https://black.readthedocs.io/en/stable/the_black_code_style/current_style.html#code-style) ensures consistent and opinionated code formatting. It automatically formats your Python code according to the Black style guide, enhancing code readability and maintainability.
 
 Key Features of Black:
@@ -31,6 +68,18 @@ Key Features of Black:
 
     Automation: Black automates the code formatting process, saving time and effort. It eliminates the need for manual formatting and reduces the likelihood of inconsistencies.
 
+### Naming Conventions
+
+- `Classes:` Class names should normally use the CapWords Convention.
+- `Functions and Variables:` Function names should be lowercase, with words separated by underscores as necessary to improve readability. Variable names follow the same convention as function names.
+
+- `Constants:` Constants are usually defined on a module level and written in all capital letters with underscores separating words.
+
+- `Non-public Methods and Instance Variables:` Use a single leading underscore (_). This is a weak "internal use" indicator.
+
+- `Strongly "private" methods and variables:` Use a double leading underscore (__). This triggers name mangling in Python.
+
+
 ### Git commit style
 
 Here’s a model Git commit message when contributing: